firebase-ios-sdk: Cannot unset/remove customMetadata keys on StorageReference objects - regression from v8

Step 0: Are you in the right place?

• For issues or feature requests related to the code in this repository file a GitHub issue.
  • If this is a feature request please use the Feature Request template.
• For general technical questions, post a question on StackOverflow with the firebase tag.
• For general (non-iOS) Firebase discussion, use the firebase-talk google group.
• For backend issues, console issues, and other non-SDK help that does not fall under one of the above categories, reach out to Firebase Support.
• Once you’ve read this section and determined that your issue is appropriate for this repository, please delete this section.

[REQUIRED] Step 1: Describe your environment

• Xcode version: 13.4
• Firebase SDK version: 9.1.0
• Installation method: CocoaPods
• Firebase Component: Storage
• Target platform(s): iOS

[REQUIRED] Step 2: Describe the problem

I’m forward-porting react-native-firebase to the v9+ SDK release here, and I’ve run into a problem with Storage.

Specifically, we attempt to mirror the firebase-js-sdk API surface area to react-native library consumers (for interoperability with web code) but we call down to underlying firebase-ios-sdk and firebase-android-sdk

firebase-js-sdk documents that to remove a customMetadata key/value pair on a StorageReference, you send in a null value to the updateMetadata call.

This worked fine with firebase-ios-sdk v8 and lower, but now it appears there are some Swift-y types / casts happening, and nulls will crash an app

I will attempt to highlight the important frames 7-16 from the boilerplate with a lot of newlines:

    updateMetadata
16:13:27.013 detox[39601] ERROR: [WS_ERROR] The app has crashed, see the details below:

Signal 6 was raised
(
        0   Detox                               0x0000000102125c15 +[NSThread(DetoxUtils) dtx_demangledCallStackSymbols] + 37
        1   Detox                               0x0000000102128670 __DTXHandleCrash + 464
        2   Detox                               0x0000000102128db1 __DTXHandleSignal + 59
        3   libsystem_platform.dylib            0x00007fff701bddfd _sigtramp + 29
        4   ???                                 0x00007fc7a2723a60 0x0 + 140495400614496
        5   libsystem_c.dylib                   0x00007fff2010b6b7 abort + 130
        6   libswiftCore.dylib                  0x00007fff30c9dc0c swift::fatalError(unsigned int, char const*, ...) + 252



        7   libswiftCore.dylib                  0x00007fff30c95f17 swift::swift_dynamicCastFailure(void const*, char const*, void const*, char const*, char const*) + 71
        8   libswiftCore.dylib                  0x00007fff30c95f8a swift::swift_dynamicCastFailure(swift::TargetMetadata<swift::InProcess> const*, swift::TargetMetadata<swift::InProcess> const*, char const*) + 106
        9   libswiftCore.dylib                  0x00007fff30c9a049 swift_dynamicCast + 249
        10  libswiftFoundation.dylib            0x00007fff599ae17a closure #2 (Swift.UnsafeMutableBufferPointer<A>, Swift.UnsafeMutableBufferPointer<B>) -> Swift.Int in static (extension in Foundation):Swift.Dictionary._forceBridgeFromObjectiveC(_: __C.NSDictionary, result: inout Swift.Optional<Swift.Dictionary<A, B>>) -> () + 762
        11  libswiftFoundation.dylib            0x00007fff599ae1f7 partial apply forwarder for closure #2 (Swift.UnsafeMutableBufferPointer<A>, Swift.UnsafeMutableBufferPointer<B>) -> Swift.Int in static (extension in Foundation):Swift.Dictionary._forceBridgeFromObjectiveC(_: __C.NSDictionary, result: inout Swift.Optional<Swift.Dictionary<A, B>>) -> () + 39
        12  libswiftFoundation.dylib            0x00007fff599b2403 Swift._NativeDictionary.init(_unsafeUninitializedCapacity: Swift.Int, allowingDuplicates: Swift.Bool, initializingWith: (Swift.UnsafeMutableBufferPointer<A>, Swift.UnsafeMutableBufferPointer<B>) -> Swift.Int) -> Swift._NativeDictionary<A, B> + 195
        13  libswiftFoundation.dylib            0x00007fff599acf90 static (extension in Foundation):Swift.Dictionary._unconditionallyBridgeFromObjectiveC(Swift.Optional<__C.NSDictionary>) -> Swift.Dictionary<A, B> + 448
        14  FirebaseStorage                     0x0000000102be888d @objc FirebaseStorage.StorageMetadata.customMetadata.setter : Swift.Optional<Swift.Dictionary<Swift.String, Swift.String>> + 93
        15  testing                             0x0000000101995c8a +[RNFBStorageCommon buildMetadataFromMap:] + 170
        16  testing                             0x00000001019970a2 -[RNFBStorageModule updateMetadata:::::] + 242




        17  CoreFoundation                      0x00007fff2040bfdc __invoking___ + 140
        18  CoreFoundation                      0x00007fff2040934f -[NSInvocation invoke] + 305
        19  CoreFoundation                      0x00007fff204095e2 -[NSInvocation invokeWithTarget:] + 70
        20  React                               0x00000001038bddb6 -[RCTModuleMethod invokeWithBridge:module:arguments:] + 2534
        21  React                               0x00000001038c227a facebook::react::invokeInner(RCTBridge*, RCTModuleData*, unsigned int, folly::dynamic const&, int, (anonymous namespace)::SchedulingContext) + 1402
        22  React                               0x00000001038c1b2c facebook::react::RCTNativeModule::invoke(unsigned int, folly::dynamic&&, int)::$_0::operator()() const + 156
        23  React                               0x00000001038c1a89 invocation function for block in facebook::react::RCTNativeModule::invoke(unsigned int, folly::dynamic&&, int) + 25
        24  DetoxSync                           0x0000000103586b27 ____detox_sync_dispatch_wrapper_block_invoke + 23
        25  libdispatch.dylib                   0x00007fff201148e4 _dispatch_call_block_and_release + 12
        26  libdispatch.dylib                   0x00007fff20115b25 _dispatch_client_callout + 8
        27  libdispatch.dylib                   0x00007fff2011c0df _dispatch_lane_serial_drain + 753
        28  libdispatch.dylib                   0x00007fff2011ccc1 _dispatch_lane_invoke + 400
        29  libdispatch.dylib                   0x00007fff2012797b _dispatch_workloop_worker_thread + 779
        30  libsystem_pthread.dylib             0x00007fff701c7fd0 _pthread_wqthread + 326
        31  libsystem_pthread.dylib             0x00007fff701c6f57 start_wqthread + 15
)

Steps to reproduce:

1- create a StorageReference with customMetadata = { removeMe: 'please'} 2- using that reference call updateMetadata({ removeMe: null }) 3- now the reference should have customMetadata with removeMe == undefined (that is, it should be been removed)

Relevant Code:

Here is our test that used to run okay on iOS (it had a failure on android we’re still pursuing, but iOS was working)

https://github.com/invertase/react-native-firebase/blob/52715959a375e8babd129baec38356b78ce80ab6/packages/storage/e2e/StorageReference.e2e.js#L403-L428

    // FIXME not working against android on emulator? it returns the string 'null' for the cleared customMetadata value
    it('should set removed customMetadata properties to null', async function () {
      if (device.getPlatform() === 'ios') {
        const storageReference = firebase.storage().ref(WRITE_ONLY_NAME);
        const metadata = await storageReference.updateMetadata({
          contentType: 'text/plain',
          customMetadata: {
            removeMe: 'please',
          },
        });


        metadata.customMetadata.removeMe.should.equal('please');


        const metadataAfterRemove = await storageReference.updateMetadata({
          contentType: 'text/plain',
          customMetadata: {
            removeMe: null,
          },
        });


        // FIXME this is failing the part that fails
        should.equal(metadataAfterRemove.customMetadata.removeMe, undefined);
      } else {
        this.skip();
      }
    });

Note that the documentation for writable metadata properties indicates you may delete them passing the empty string: https://firebase.google.com/docs/storage/ios/file-metadata#update_file_metadata

On Android (and web) writable metadata is removed by passing null https://firebase.google.com/docs/storage/android/file-metadata#update_file_metadata https://firebase.google.com/docs/storage/web/file-metadata#update_file_metadata

No mention is made on how to remove customMetadata keys.

If you attempt sending an empty string in for a key with customMetadata, that just results in the value being the empty string, the value is not removed, so there may be something to think about for API consistency.

But in the past at least, sending in a null for a customMetadata key removed it fine.

Here’s the iOS code that does the update:

https://github.com/invertase/react-native-firebase/blob/52715959a375e8babd129baec38356b78ce80ab6/packages/storage/ios/RNFBStorage/RNFBStorageModule.m#L145-L156

  FIRStorageReference *storageReference = [self getReferenceFromUrl:url app:firebaseApp];
  FIRStorageMetadata *storageMetadata = [RNFBStorageCommon buildMetadataFromMap:metadata];


  [storageReference
      updateMetadata:storageMetadata
          completion:^(FIRStorageMetadata *_Nullable metadata, NSError *_Nullable error) {
            if (error != nil) {
              [self promiseRejectStorageException:reject error:error];
            } else {
              resolve([RNFBStorageCommon metadataToDict:metadata]);
            }
          }];

…and the buildMetadataFromMap method is

https://github.com/invertase/react-native-firebase/blob/52715959a375e8babd129baec38356b78ce80ab6/packages/storage/ios/RNFBStorage/RNFBStorageCommon.m#L395-L399

+ (FIRStorageMetadata *)buildMetadataFromMap:(NSDictionary *)metadata {
  FIRStorageMetadata *storageMetadata = [[FIRStorageMetadata alloc] initWithDictionary:metadata];
  storageMetadata.customMetadata = [metadata[@"customMetadata"] mutableCopy];
  return storageMetadata;
}

That storageMetadata.customMetadata = line is the crash, when the FIRStorageMetadata is initialized with an NSDictionary that contains nulls

I understand that nil NSNull null etc can be bothersome in Objective-C and for interop, and who knows what the react-native bridge is translating ‘null’ from javascript in to at the objective-c layer, right? so I chucked this line in to the code:

  NSLog(@"FIRStorageMetadata removeMe key %@", metadata[@"customMetadata"]);

And it spits out this, which seems to indicate it is an NSNull if I understand correctly.

2022-05-26 17:23:08.876 Df testing[44279:500cc] FIRStorageMetadata removeMe key {
    removeMe = "<null>";
}

I tried changing the swfit StorageMetadata.swift file from this repo a bit, playing with type signatures etc but if my objective-c skills are lacking (they are) my Swift skills are almost non-existent, and making interoperable optional/nullable types is beyond me unfortunately

Help?