-
-
Notifications
You must be signed in to change notification settings - Fork 538
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Request body with Protobuf binary data is corrupted #1442
Comments
Related issue:#929 |
Opened issue in msw-storybook-addon, but whilst writing that I realized this:
|
As mentioned above, added a test for expected behaviour. Considering its the exact same error this means that I likely have been using 0.47 in storybook, even though the addon says 0.35+ |
It's not msw's fault. It's the encoding and decoding. It also has nothing to do with protobuf. The following example fails: test('fails', async () => {
const input = new Uint8Array([138, 1, 6, 10, 4, 10, 2, 32, 1])
const text = new util.TextDecoder().decode(input)
const bytes = new Uint8Array(new util.TextEncoder().encode(text).buffer)
expect(bytes).toEqual(input)
}) |
It's quite possible that I made a mistake in this attempt at reproducing the issue. I'm also quite aware this random array of values isn't protobuf in itself. But it does come from protobuf and figured it might help make the issue more 'real' :) Anyway, if I input 9 characters as a uint8array (iirc) then text().length is 9, arrayBuffer().length is 11 So something in that array buffer is doing something encoding wise and I can't figure out how to properly convert text to a buffer. Is there an even more 'raw' way to access the body? I simply want the exact same data to come out somehow |
for completeness sake, here's the actual process const login = async (req, res, ctx) => {
const user = User.decode(
new Uint8Array(await req.arrayBuffer()) // arrayBuffer -> UInt8Array -> object
);
// panic, if the req.arrayBuffer has unexpected characters, see my included example. Not all messages, just certain messages
return res(
ctx.status(200),
ctx.set("Content-Type", "application/protobuf"),
ctx.body(Token.encode(
{
token:
"eyJ(..)KrmD",
},
).finish();) // object -> UInt8Array
); It;s quite possible this isn't the fault of MSW, although I would expected the |
Hey, @DGollings. Thanks for reporting this. We've done some fundamental improvements to how MSW handles |
Np, how stable is that branch in #1436 ? Are my assumptions in #1442 (comment) correct? If so, I'll happily generate a new mockServiceWorker.js and try to help test it |
The branch is stable but you won't be able to build it just yet. It has a few dependencies on other pull requests/packages to be merged/released before it can be built correctly. Also, it's not as much about the worker script but rather about how we handle requests/responses internally (well, the changes affect the worker a little). I don't think that swapping a worker script would be enough to test whether it fixes this issue. Also, it's a huge breaking change with its own migration guidelines (more on that once we publish a release candidate). |
I am also hitting this exact same issue. Having dug into it a bit, the issues is the following (and maybe #1436 fixes it? I wasn't sure where begin looking since that diff contains 265 changed files). Below is a snippet of code from the generated const clientMessage = await sendToClient(client, {
type: 'REQUEST',
payload: {
id: requestId,
url: request.url,
method: request.method,
headers: Object.fromEntries(request.headers.entries()),
cache: request.cache,
mode: request.mode,
credentials: request.credentials,
destination: request.destination,
integrity: request.integrity,
redirect: request.redirect,
referrer: request.referrer,
referrerPolicy: request.referrerPolicy,
body: await request.text(), // this is the problematic line
bodyUsed: request.bodyUsed,
keepalive: request.keepalive,
},
}) Inside of the msw handler, when you call
I don't have a great understanding of the limitations on what structure the data has to be in for a service worker, but assuming it has to be text, could you do something like base64 encode the byte array and then undo it in the correspond
As of now, the request body (when you see it in the handler function) will be corrupt due to the |
Didn't immediately think of this, even though its a pretty standard thing to do whenever handling binary data. Is there a technical limitation/reason why this wouldn't be possible? |
Small experience report: I was also having issues with corrupted request data in a PDF file upload mock endpoint (using version 0.46.1). The corruption looked just like the one described in this issue. After upgrading to msw@next there is no corruption anymore :-) |
Released: v2.0.0 🎉This has been released in v2.0.0! Make sure to always update to the latest version ( Predictable release automation by @ossjs/release. |
Hey, folks. I tried reproducing this on MSW 2.x and the issue seems to be resolved by relying on the standard Fetch API. I also confirm that the submitted test is now passing. I'm about to merge it. Upgrade to the newest MSW version to have this fixed. Thanks. |
Prerequisites
Environment check
msw
versionBrowsers
Chromium (Chrome, Brave, etc.), Firefox
Reproduction repository
not easily possible
Reproduction steps
Good news, I saw some issues with people having problems(?) mocking binary protobuf messages. I can confirm it works though for Twirp (which is like gRPC but simplified). Simply encode and decode with a properly set header
Well, mostly:
My input
I can encode/decode without issues before sending to msw, but in the mock itself it turns into either:
or with a little (unsuccessful) hacking:
usingText[0] = 138
'fixes it', but unfortunately doesn't work in all casesNotice the values
[239, 191, 189, ...]
, that's a unicode replacement character. As in, the arrayBuffer has been at some point converted from raw data to a string and 'back' (?)A likely important note: I use
msw-storybook-addon
EDIT: whilst writing this I think I realize the problem. That addon has version
set. Which doesn't contain the fixes I think I need.
I'll likely have to open an issue there. But leaving this here for completeness/cross reference
Current behavior
arrayBuffer is stringified
Expected behavior
arrayBuffer is completely untouched
The text was updated successfully, but these errors were encountered: