ionic-team / capacitor

Build cross-platform Native Progressive Web Apps for iOS, Android, and the Web ⚡️
https://capacitorjs.com
MIT License
11.67k stars 989 forks source link

[Bug]: Percent-encoding of url search parameters broken for patched fetch / xhr requests through CapacitorHttp #7523

Closed michaelwolz closed 1 week ago

michaelwolz commented 1 month ago

Capacitor Version

💊   Capacitor Doctor  💊 

Latest Dependencies:

  @capacitor/cli: 6.1.0
  @capacitor/core: 6.1.0
  @capacitor/android: 6.1.0
  @capacitor/ios: 6.1.0

Installed Dependencies:

  @capacitor/cli: 6.1.0
  @capacitor/core: 6.1.0
  @capacitor/android: 6.1.0
  @capacitor/ios: 6.1.0

Other API Details

No response

Platforms Affected

Current Behavior

All patched fetch and xhr requests using any of the request methods GET, HEAD, OPTIONS and TRACE do not correctly consider percent-encoding of url search parameters. Especially it breaks the encoding of properly encoded url search parameters.

Example

Let's consider the following fetch call: await fetch(`https://example.com?param=${encodeURIComponent(";:@&=+$,/?%#[]")}`);

The resulting URL to call will then look like this on the Javascript side and work fine in the browser: https://example.com?param=%3B%3A%40%26%3D%2B%24%2C%2F%3F%25%23%5B%5D

On Android the called URL will look like this though: https://example.com/?param=!'();:@&=+$,/?%

and on iOS like this (oddly enough, iOS somehow managed to encode the "%" character itself correctly, but I assume Swift handles this somehow?): https://example.com/?param=!'();:@&=+$,/?%25

See intercepted network traffic via mitmproxy: image

Expected Behavior

Patched fetch and xhr request should respect url encoding.

Project Reproduction

https://github.com/michaelwolz/capacitor-tests

Additional Information

I tried to investigate the root cause of this problem and found that PR #7273 introduced some logic to correctly handle the colon character : in hostnames for proxied urls in order to be able to correctly use it with different ports. For this the hostname of the requested url is encoded in the createProxyUrl method in the native-bridge.

When we enable the CapacitorHttp plugin all requests of type GET, HEAD, OPTIONS and TRACE will go through the monkey patched version of fetch (or xhr, the behavior is the applies to both) which will then create the proxy url and pass it on to the original browser fetch method (via win.CapacitorWebFetch). This request is then being intercepted and processed by the native code in WebViewLocalServer on Android and WebViewAssetHandler on iOS.

The actual issue happens then in the respective handler function (handleCapacitorHttpRequest) here and here. At this point, the entire URL string is decoded, while in the native bridge on the JS side, only the host was encoded. This will remove any previous encoding applied to the URL except for the host. I think a solution could be encoding the entire URL in the createProxyUrl method of the native-bridge. This might double encode search params, but decoding should give the correct URL again, assuming the native decoder functions are compatible with the JS encode version. I will test this and ~possibly~ submit a PR (PR: #7527), but I'm not sure if there might be any side effects which I haven't considered yet and would appreciate any feedback.

Edit: Actually this will most certainly also break encoded pathnames and the url hash because the same logic applies here.

ionitron-bot[bot] commented 1 month ago

This issue has been labeled as type: bug. This label is added to issues that that have been reproduced and are being tracked in our internal issue tracker.