Skip to content

Commit fd6fcf3

Browse files
authored
[Storage] Improve how GetFile handles paths (#1782)
* [Storage] Improve how GetFile handles paths * Update based on feedback * Update storage_reference_ios.mm * Update storage_reference_ios.mm * Revert "Update storage_reference_ios.mm" This reverts commit c429475.
1 parent 4086d40 commit fd6fcf3

File tree

3 files changed

+45
-1
lines changed

3 files changed

+45
-1
lines changed

release_build_files/readme.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -613,6 +613,10 @@ workflow use only during the development of your app, not for publicly shipping
613613
code.
614614

615615
## Release Notes
616+
### Upcoming
617+
- Changes
618+
- Storage (iOS): Handle absolute paths being provided to GetFile. (#1724)
619+
616620
### 13.0.0
617621
- Changes
618622
- General (Android): Update to Firebase Android BoM version 34.0.0.

storage/integration_test/src/integration_test.cc

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -657,6 +657,28 @@ TEST_F(FirebaseStorageTest, TestPutFileAndGetFile) {
657657
EXPECT_NE(future.result(), nullptr);
658658
EXPECT_EQ(*future.result(), kSimpleTestFile.size());
659659

660+
std::vector<char> buffer(kSimpleTestFile.size());
661+
FILE* file = fopen(path.c_str(), "rb");
662+
EXPECT_NE(file, nullptr);
663+
size_t bytes_read = std::fread(&buffer[0], 1, kSimpleTestFile.size(), file);
664+
EXPECT_EQ(bytes_read, kSimpleTestFile.size());
665+
fclose(file);
666+
EXPECT_EQ(memcmp(&kSimpleTestFile[0], &buffer[0], buffer.size()), 0);
667+
}
668+
// Test GetFile without the file prefix to ensure we can download to a file.
669+
{
670+
std::string path = PathForResource() + kGetFileTestFile;
671+
// Try the direct path, which should also work.
672+
std::string file_path = path;
673+
674+
LogDebug("Saving to local file: %s", path.c_str());
675+
676+
firebase::Future<size_t> future =
677+
RunWithRetry<size_t>([&]() { return ref.GetFile(file_path.c_str()); });
678+
WaitForCompletion(future, "GetFile");
679+
EXPECT_NE(future.result(), nullptr);
680+
EXPECT_EQ(*future.result(), kSimpleTestFile.size());
681+
660682
std::vector<char> buffer(kSimpleTestFile.size());
661683
FILE* file = fopen(path.c_str(), "rb");
662684
EXPECT_NE(file, nullptr);

storage/src/ios/storage_reference_ios.mm

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,25 @@
142142
// Cache a copy of the impl and storage, in case this is destroyed before the thread runs.
143143
FIRStorageReference* my_impl = impl();
144144
StorageInternal* storage = storage_;
145-
NSURL* local_file_url = [NSURL URLWithString:@(path)];
145+
NSString* path_string = @(path);
146+
if (path_string.length == 0) {
147+
future_impl->Complete(handle, kErrorUnknown, "Path cannot be empty.");
148+
return GetFileLastResult();
149+
}
150+
NSURL* local_file_url = nil;
151+
if ([path_string hasPrefix:@"file://"]) {
152+
// If it starts with the prefix, load it assuming a URL string.
153+
local_file_url = [NSURL URLWithString:path_string];
154+
} else {
155+
// Otherwise, assume it is a file path.
156+
local_file_url = [NSURL fileURLWithPath:path_string];
157+
}
158+
// If we still failed to convert the path, error out.
159+
if (local_file_url == nil) {
160+
future_impl->Complete(handle, kErrorUnknown,
161+
"Unable to convert provided path to valid URL");
162+
return GetFileLastResult();
163+
}
146164
util::DispatchAsyncSafeMainQueue(^() {
147165
FIRStorageDownloadTask *download_task;
148166
{

0 commit comments

Comments
 (0)