Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: Boundary is not extracted correctly from multipart/form-data requests when using CapacitorHttp #7621

Open
2 of 3 tasks
michaelwolz opened this issue Aug 14, 2024 · 9 comments
Assignees
Labels

Comments

@michaelwolz
Copy link
Contributor

michaelwolz commented Aug 14, 2024

Capacitor Version

💊   Capacitor Doctor  💊

Latest Dependencies:

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

Installed Dependencies:

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

The extraction of the value of the boundary of multipart/form-data requests fails for some requests due to the value being surrounded by double quotes, which is allowed and happens occasionally (See MDN Reference which utilizes double quotes for the boundary in their example: https://developer.mozilla.org/en-US/docs/Web/HTTP/Methods/POST#example). The current implementation only works with no surroundings of the boundary value and otherwise just includes them in the actual request body which will then fail on the server side where the serve will not be able to parse the request body.

In addition to that, the whole extraction currently just uses the last key of the Content-Type header which might not necessarily be the boundary but could also be something such as charset (see example implementation or MDN reference).

Other API Details

No response

Platforms Affected

  • iOS
  • Android
  • Web

Current Behavior

Let's consider the following request (assuming CapacitorHttp is active)

const formData = new FormData();
formData.append('key', 'value');

await fetch(url, { method: 'POST', body, 
  headers: { 'Content-Type': 'multipart/form-data; boundary="boundary"; charset=utf8' }
});

Because of CapacitorHttp is currently taking the last key of the Content-Type header as boundary the actual request body sent by CapacitorHttp will then look like:

--utf8
Content-Disposition: form-data; name="key"

value
--utf8--

The same request without the additional charset key but enclosed in double quotes: ( 'Content-Type': 'multipart/form-data; boundary="boundary"') will look like this:

--"boundary"
Content-Disposition: form-data; name="key"

value
--"boundary"--

The server will however read the boundary without quotes from the header and thus will be unable to parse the body.

Expected Behavior

For both cases the request body should look like this:

--boundary
Content-Disposition: form-data; name="key"

value
--boundary--

Project Reproduction

https://github.com/michaelwolz/multipart-form-data-not-working-on-ios

Additional Information

(Non-)working example can be found here: https://github.com/michaelwolz/multipart-form-data-not-working-on-ios/blob/master/src/js/capacitor-welcome.js#L76-L94

You install the current official version and it'll fail while working with the provided fix.

Pull-Request fixing the problem by accepting arbitrary position of the boundary and ignoring quotes: #7518

Fairly sure that the described behavior is also the origin of the following issues:

@michaelwolz michaelwolz changed the title [Bug]: boundary is not extracted correctly from multipart/form-data requests when using CapacitorHttp [Bug]: Boundary is not extracted correctly from multipart/form-data requests when using CapacitorHttp Aug 14, 2024
@markemer markemer self-assigned this Aug 19, 2024
@VashLT

This comment was marked as off-topic.

@fobaz

This comment was marked as off-topic.

@kapocius

This comment was marked as off-topic.

@peterrmah

This comment was marked as abuse.

@grzegorzCieslik95
Copy link

Hey, My team is waiting too for this update. Any information about when we can expect this update on production?

@koczka
Copy link

koczka commented Oct 29, 2024

I'm also having the same issue.

@peterrmah

This comment was marked as abuse.

@markemer
Copy link
Member

Hey @jcesarmobile & team, I'm running into the same issue after upgrading from Capacitor 5 to 6. With the Google Play Store forcing the move to Android target version SDK 34, upgrading to Capacitor 6 seems to be a requirement. Unfortunately, this is blocking us us from submitting app updates, which has become an increasingly urgent matter for business and technical reasons.

I know we're not the only ones dealing with this, as I've seen other tickets about the issue but they were closed due to inactivity. We're really hoping for some guidance or a fix soon, as this has become a critical blocker for the community.

Thanks for all your hard work! Any update on this would be super helpful.

Relevant links: #7498 (comment) #5498

It seems my previous comment was marked as "abuse" despite having chosen my wording very carefully and keeping an optimistic tone. A few other comments before me were also marked as "off topic" even by simply asking if there is an update on the issue.

There seems to be a growing number of users in the Capacitor community experiencing this blocker of a core issue. It would be very much appreciated for someone from the Capacitor team can give us an update in good faith. @markemer

That I think is the tagging, it may even be automated. Try to avoid tagging Julio and I, but this is on our radar for sure, but I cannot get a definitive date for you right now.

@michaelwolz
Copy link
Contributor Author

michaelwolz commented Oct 30, 2024

For anyone who is really blocked by this, you might consider patching the plugin following this guide here, for example: https://capawesome.io/blog/how-to-patch-a-capacitor-plugin/.

You just need to adapt two files according to my PR #7518:

  • android/capacitor/src/main/java/com/getcapacitor/plugin/util/CapacitorHttpUrlConnection.java
  • ios/Capacitor/Capacitor/Plugins/CapacitorUrlRequest.swift

I know this is kind of a hacky solution, but on the other hand, we have been using the patched version in production code for a few months now with success. Still, it would be great to have this verified by the Capacitor team, as it's entirely possible that I missed something with my changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

8 participants