Add error for attempt to upload directory (#5750)

Author: ncooke3

FirebaseStorage/CHANGELOG.md

@@ -1,6 +1,8 @@
 # 3.7.0
 - [fixed] Fixed a crash when listAll() was called at the root location. (#5772)
 
+- [added] Added a check to FIRStorageUploadTask's `putFile:` to check if the passed in `fileURL` is a directory, and provides a clear error if it is. (#5750)
+
 # 3.6.1
 - [fixed] Fix a rare case where a StorageTask would call its completion callbacks more than
   once. (#5245)

FirebaseStorage/Sources/FIRStorageUploadTask.m

@@ -193,11 +193,14 @@ - (BOOL)isContentToUploadValid:(NSError **)outError {
   }
 
   NSError *fileReachabilityError;
-  if (![_fileURL checkResourceIsReachableAndReturnError:&fileReachabilityError]) {
+  if (![_fileURL checkResourceIsReachableAndReturnError:&fileReachabilityError] ||
+      ![self fileURLisFile:_fileURL]) {
     if (outError != NULL) {
       NSMutableDictionary *userInfo = [NSMutableDictionary dictionaryWithCapacity:2];
-      userInfo[NSLocalizedDescriptionKey] =
-          [NSString stringWithFormat:@"File at URL: %@ is not reachable.", _fileURL.absoluteString];
+      userInfo[NSLocalizedDescriptionKey] = [NSString
+          stringWithFormat:@"File at URL: %@ is not reachable. "
+                           @"Ensure file URL is not a directory, symbolic link, or invalid url.",
+                           _fileURL.absoluteString];
 
       if (fileReachabilityError) {
         userInfo[NSUnderlyingErrorKey] = fileReachabilityError;
@@ -257,4 +260,12 @@ - (void)resume {
   }];
 }
 
+#pragma mark - Private Helpers
+
+- (BOOL)fileURLisFile:(NSURL *)fileURL {
+  NSNumber *isFile = [NSNumber numberWithBool:NO];
+  [fileURL getResourceValue:&isFile forKey:NSURLIsRegularFileKey error:nil];
+  return [isFile boolValue];
+}
+
 @end

FirebaseStorage/Tests/Integration/Resources/HomeImprovement.numbers/About.txt

@@ -0,0 +1 @@
+The `HomeImprovement.numbers` directory exists to test Firebase Storage's ability to fail gracefully when one attempts to upload a directory. Since git doesn't track track empty directories. This .txt file exists to provide context and make git track the folder.

FirebaseStorage/Tests/Unit/FIRStorageReferenceTests.m

@@ -180,7 +180,9 @@ - (void)testReferenceWithNonExistentFileFailsWithCompletion {
         XCTAssertEqualObjects(error.domain, FIRStorageErrorDomain);
         XCTAssertEqual(error.code, FIRStorageErrorCodeUnknown);
         NSString *expectedDescription = [NSString
-            stringWithFormat:@"File at URL: %@ is not reachable.", dummyFileURL.absoluteString];
+            stringWithFormat:@"File at URL: %@ is not reachable. "
+                             @"Ensure file URL is not a directory, symbolic link, or invalid url.",
+                             dummyFileURL.absoluteString];
         XCTAssertEqualObjects(error.localizedDescription, expectedDescription);
       }];
 
@@ -204,7 +206,10 @@ - (void)testReferenceWithNilFileURLFailsWithCompletion {
 
         XCTAssertEqualObjects(error.domain, FIRStorageErrorDomain);
         XCTAssertEqual(error.code, FIRStorageErrorCodeUnknown);
-        NSString *expectedDescription = @"File at URL: (null) is not reachable.";
+        NSString *expectedDescription = [NSString
+            stringWithFormat:@"File at URL: %@ is not reachable. "
+                             @"Ensure file URL is not a directory, symbolic link, or invalid url.",
+                             dummyFileURL.absoluteString];
         XCTAssertEqualObjects(error.localizedDescription, expectedDescription);
       }];
 

FirebaseStorageSwift.podspec

@@ -39,7 +39,8 @@ Firebase Storage provides robust, secure file uploads and downloads from Firebas
     int_tests.requires_app_host = true
     # Resources are shared with FirebaseStorage's integration tests.
     int_tests.resources = 'FirebaseStorage/Tests/Integration/Resources/1mb.dat',
-                          'FirebaseStorage/Tests/Integration/Resources/GoogleService-Info.plist'
+                          'FirebaseStorage/Tests/Integration/Resources/GoogleService-Info.plist',
+                          'FirebaseStorage/Tests/Integration/Resources/HomeImprovement.numbers'
     int_tests.dependency 'FirebaseAuth', '~> 6.5'
   end
 end

FirebaseStorageSwift/Tests/Integration/StorageIntegration.swift

@@ -256,6 +256,18 @@ class StorageIntegration: XCTestCase {
     waitForExpectations()
   }
 
+  func testAttemptToUploadDirectoryShouldFail() throws {
+    // This `.numbers` file is actually a directory.
+    let fileName = "HomeImprovement.numbers"
+    let bundle = Bundle(for: StorageIntegration.self)
+    let fileURL = try XCTUnwrap(bundle.url(forResource: fileName, withExtension: ""),
+                                "Failed to get filePath")
+    let ref = storage.reference(withPath: "ios/public/" + fileName)
+    ref.putFile(from: fileURL) { result in
+      self.assertResultFailure(result)
+    }
+  }
+
   func testPutFileWithSpecialCharacters() throws {
     let expectation = self.expectation(description: #function)
 
@@ -607,4 +619,14 @@ class StorageIntegration: XCTestCase {
       XCTFail("Unexpected error \(error)")
     }
   }
+
+  private func assertResultFailure<T>(_ result: Result<T, Error>,
+                                      file: StaticString = #file, line: UInt = #line) {
+    switch result {
+    case let .success(value):
+      XCTFail("Unexpected success with value: \(value)")
+    case let .failure(error):
+      XCTAssertNotNil(error, file: file, line: line)
+    }
+  }
 }