From b1ae0dc5e2d2f96dfecab0c828fbfd4c33074779 Mon Sep 17 00:00:00 2001 From: Thomas P Date: Fri, 18 Dec 2020 12:16:10 +0100 Subject: [PATCH 1/3] FIX: [Storage] Don't try resolving the promise twice if an error occurs in listAll() See https://github.com/firebase/firebase-ios-sdk/blob/14764b8d60a6ad023d8fa5b7f81d42378d92e6fe/FirebaseStorage/Sources/FIRStorageReference.m#L417 --- packages/storage/ios/RNFBStorage/RNFBStorageModule.m | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/packages/storage/ios/RNFBStorage/RNFBStorageModule.m b/packages/storage/ios/RNFBStorage/RNFBStorageModule.m index a3a92a87dd..08dde18b74 100644 --- a/packages/storage/ios/RNFBStorage/RNFBStorageModule.m +++ b/packages/storage/ios/RNFBStorage/RNFBStorageModule.m @@ -194,7 +194,16 @@ - (void)invalidate { ) { FIRStorageReference *storageReference = [self getReferenceFromUrl:url app:firebaseApp]; + __block bool alreadyCompleted = false; + id completionBlock = ^(FIRStorageListResult *result, NSError *error) { + // This may be called multiple times if an error occurs + // Make sure we won't try to resolve the promise twice in this case + // See https://github.com/firebase/firebase-ios-sdk/blob/14764b8d60a6ad023d8fa5b7f81d42378d92e6fe/FirebaseStorage/Sources/FIRStorageReference.m#L417 + if(alreadyCompleted) { + return; + } + alreadyCompleted = true; if (error != nil) { [self promiseRejectStorageException:reject error:error]; } else { From dcb7aceecd4b24904a3f9234d141506ebf3263de Mon Sep 17 00:00:00 2001 From: Thomas P Date: Fri, 18 Dec 2020 12:43:22 +0100 Subject: [PATCH 2/3] TST: Make sure listAll behaves as expected when the user is not authorized --- packages/storage/e2e/StorageReference.e2e.js | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/packages/storage/e2e/StorageReference.e2e.js b/packages/storage/e2e/StorageReference.e2e.js index 76edcc68fb..2d356ff79d 100644 --- a/packages/storage/e2e/StorageReference.e2e.js +++ b/packages/storage/e2e/StorageReference.e2e.js @@ -346,6 +346,20 @@ describe('storage() -> StorageReference', function() { result.prefixes.length.should.be.greaterThan(0); result.prefixes[0].constructor.name.should.eql('StorageReference'); }); + + it('should not crash if the user is not allowed to list the directory', async function() { + const storageReference = firebase.storage().ref('/forbidden'); + try { + await storageReference.listAll(); + return Promise.reject(new Error('listAll on a forbidden directory succeeded')); + } catch (error) { + error.code.should.equal('storage/unauthorized'); + error.message.should.equal( + '[storage/unauthorized] User is not authorized to perform the desired action.', + ); + return Promise.resolve(); + } + }); }); describe('updateMetadata', function() { From 6913a614b53ed6b6b482d178c415ce3cdc1eb291 Mon Sep 17 00:00:00 2001 From: Mike Hardy Date: Fri, 18 Dec 2020 09:13:26 -0500 Subject: [PATCH 3/3] Apply suggestions from code review --- packages/storage/ios/RNFBStorage/RNFBStorageModule.m | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/storage/ios/RNFBStorage/RNFBStorageModule.m b/packages/storage/ios/RNFBStorage/RNFBStorageModule.m index 08dde18b74..012367193e 100644 --- a/packages/storage/ios/RNFBStorage/RNFBStorageModule.m +++ b/packages/storage/ios/RNFBStorage/RNFBStorageModule.m @@ -199,8 +199,8 @@ - (void)invalidate { id completionBlock = ^(FIRStorageListResult *result, NSError *error) { // This may be called multiple times if an error occurs // Make sure we won't try to resolve the promise twice in this case - // See https://github.com/firebase/firebase-ios-sdk/blob/14764b8d60a6ad023d8fa5b7f81d42378d92e6fe/FirebaseStorage/Sources/FIRStorageReference.m#L417 - if(alreadyCompleted) { + // TODO - remove pending resolution of https://github.com/firebase/firebase-ios-sdk/issues/7197 + if (alreadyCompleted) { return; } alreadyCompleted = true;