fix: preserve binary request bodies over base64 - #138
Closed
PradyumnaShome wants to merge 1 commit into
Closed
Conversation
|
@PradyumnaShome is attempting to deploy a commit to the Margelo Team on Vercel. A member of the Team first needs to authorize it. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ArrayBufferand typed-array request bodies as base64 at the existing string wire boundaryURLSearchParams, and form-data handling unchangedWhy
Binary request bodies are normalized in JavaScript today, but
bodyBytesis then left unset. Even if it is provided, Android currently uploads a one-byte placeholder and iOS does not attach it to the request. This can drop or corrupt binary uploads.This draft is an interim correctness fix: it intentionally retains the current
bodyBytes?: stringbase64 transport and does not change the Nitro spec or generated ABI.Tests
bun --cwd packages/react-native-nitro-fetch test --runInBand src/__tests__/fetch.test.tsbun --cwd packages/react-native-nitro-fetch test --runInBandbun typecheckbun lint(0 errors; 2 pre-existingno-shadowwarnings)git diff --checkLimitations / follow-up
The new coverage is a JavaScript contract test for the current base64 wire format. I did not run an iOS or Android app build in this checkout, so native decoding is not exercised by an integration test here.
#112 proposes the broader
ArrayBuffertransport and generated binding changes. That remains the preferred follow-up; this draft is deliberately narrower so the request-body correctness fix can be reviewed independently without an ABI change.