Fix: URLSearchParams dropped when body of XHR

[ajaska]

Note: Code untested, help wanted!

Summary

Right now, if you set a URLSearchParams as the body of a XMLHTTPRequest, it is silently dropped. Support for this feature is in the whatwg fetch spec and currently supported in browsers.

react-native's Code

Before a request gets sent to native code, the body gets run through a conversion function which lacks support for URLSearchParams.

Since the body doesn't get converted, it ends up being dropped (silently) in the native code:

// Nothing in the data payload, at least nothing we could understand anyway.
// Ignore and treat it as if it were null.

https://github.com/facebook/react-native/blob/master/Libraries/Network/RCTNetworking.mm#L337-L412

The Fix

I see two options here:

1. [Probably not this one] You could just convert the URLSearchParams, calling .toString() and then treating it as if it were a regular string. The diff would be short, but the content-type header would incorrectly default to text/plain;charset=UTF-8 instead of application/x-www-form-urlencoded;charset=UTF-8. External callers could still pass the correct headers (and whatwg-fetch, for example, would in fact take care of that for you).
2. From convertRequestBody, return an object { urlSearchParams: body.toString() } and update processDataForHTTPQuery to accept this as an input type.

As an additional feature, it might be nice to get some sort of warning or error when an unsupported/non-processable object is passed in, instead of silently ignoring it.

My Code

I'm going to be real with you: I don't even know what language a .mm file is. I don't have an Apple computer to run the tests on, and I don't even know if this code is also run on Android or there's a separate file for that.

That said, I've included my best impression of what this diff should look like. (Using approach 2.) Would appreciate support from any other contributors.

Edit: Is this the corresponding Android code?

Workaround for anyone coming in from the internet

A current workaround found in the wild for this issue seems to be to call .toString() on the URLSearchParams yourself.

However, I discovered this issue while using an auto-generated API client library and implementing this workaround will be pretty hacky.

Reproducible Snack

Here's a reproducible Snack showing that it is broken on the iOS and Android devices: https://snack.expo.io/z4N9PcIR7

Note that the snack doesn't work on the web simulator because of CORS restrictions. However, you can check the network tab to see that in web mode, the data is being sent (because web uses the browser's XMLHttpRequest instead of react-native's)

Changelog

[General] [Fixed] - Fix body of XMLHttpRequest being dropped when it is a URLSearchParams

Test Plan

HELP WANTED HERE

[facebook-github-bot]

Hi @ajaska!

Thank you for your pull request and welcome to our community.We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file.

In order for us to review and merge your code, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

[facebook-github-bot]

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	6,738,978	-20
android	hermes	armeabi-v7a	6,404,047	-20
android	hermes	x86	7,121,156	-23
android	hermes	x86_64	7,011,711	-3
android	jsc	arm64-v8a	8,907,312	69
android	jsc	armeabi-v7a	8,564,739	38
android	jsc	x86	8,732,589	47
android	jsc	x86_64	9,308,780	59

Base commit: 03385ac

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
ios	-	universal	10,676,080	9,866,512

Base commit: 03385ac

[github-actions]

This PR is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 7 days.

[github-actions]

This PR was closed because it has been stalled for 7 days with no activity.