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

Got request is silently failing on compressed data when an interceptor is set up #308

Closed
OlivierRiberi opened this issue Nov 24, 2022 · 5 comments · Fixed by #318
Closed

Comments

@OlivierRiberi
Copy link

Hi,

I'm using Got and it seems like the request is silently failing while fetching compressed data (like brotli or gzip).
In the request option, Got provides a decompress option (default: true) that use decompress-response internally.
Of course, everything is working fine when the interceptor is not set up.

Thanks

Repro: https://replit.com/@qyfjs9bfwd/Interceptor-and-Got-with-compressed-data

import got from 'got'

import { BatchInterceptor } from '@mswjs/interceptors'
import { ClientRequestInterceptor } from '@mswjs/interceptors/lib/interceptors/ClientRequest/index.js'
import { XMLHttpRequestInterceptor } from '@mswjs/interceptors/lib/interceptors/XMLHttpRequest/index.js'

const interceptor = new BatchInterceptor({
  name: 'my-interceptor',
  interceptors: [
    new ClientRequestInterceptor(),
    new XMLHttpRequestInterceptor(),
  ]
})

interceptor.apply()

interceptor.on('request', (request) => {
  console.log('interceptor listener')
})

// Working
const resp = await got('https://httpbin.org/brotli', { decompress: false })
console.log(resp.body)

// Not working
const resp2 = await got('https://httpbin.org/brotli')
console.log(resp2.body) // Does not show up

// Both requests are working if interceptor.apply() is commented out
@blaenk
Copy link
Contributor

blaenk commented Dec 6, 2022

I was about to create a new ticket but I think I'm running into this as well.

I am not using Got, but a site's responses have e.g. Content-Encoding: br or gzip or deflate and I get all kinds of exceptions thrown like:

Uncaught RangeError: Failed to construct 'TextDecoder': The encoding label provided ('br') is invalid.
Uncaught RangeError: Failed to construct 'TextDecoder': The encoding label provided ('gzip') is invalid.

And so on.

While searching around to figure out what the issue might be, it seems like it might be possible to decompress/compress also with native browser APIs using DecompressionStream and CompressionStream:

https://gist.github.com/Explosion-Scratch/357c2eebd8254f8ea5548b0e6ac7a61b

function compress(string, encoding) {
  const byteArray = new TextEncoder().encode(string);
  const cs = new CompressionStream(encoding);
  const writer = cs.writable.getWriter();
  writer.write(byteArray);
  writer.close();
  return new Response(cs.readable).arrayBuffer();
}

function decompress(byteArray, encoding) {
  const cs = new DecompressionStream(encoding);
  const writer = cs.writable.getWriter();
  writer.write(byteArray);
  writer.close();
  return new Response(cs.readable).arrayBuffer().then(function (arrayBuffer) {
    return new TextDecoder().decode(arrayBuffer);
  });
}

@blaenk
Copy link
Contributor

blaenk commented Dec 8, 2022

I was investigating some more and I noticed that simply passing undefined to decodeBuffer() instead of the strings br, gzip, etc., worked perfectly fine. This is on browser by the way, so maybe different from this issue after all.

It actually seems like using the Content-Encoding header and passing it straight through to TextEncoder doesn't quite make sense because Content-Encoding is typically about compression, whereas TextEncoder's parameter is about character encoding like utf8.

I'm gonna offer a PR to remove that feature.

@kettanaito
Copy link
Member

@blaenk, you're right on this. We are mistakingly forwarding response content encoding to the text decoder of buffer bodies in XMLHttpRequest. Thanks for opening a fix in #318!

Since the original issue is related to compression, I believe that your investigation and the fix will close this issue.

kettanaito added a commit that referenced this issue Dec 9, 2022
)

- Fixes #308 

Thank you so much for working on this project, it is immensely useful!

TLDR: I am using the browser interceptors on a site that is serving
responses with `Content-Encoding: br` or `gzip` or `deflate` etc. This
breaks every request since `Content-Encoding` is typically for
compression algorithms, like `gzip`, like this site responds with, but
`TextDecoder` takes a character encoding, like `utf-8`. This breaks
every request even if nothing is done by the user because the library
logs the response, which implicitly calls `responseText()`. Avoiding
passing the `Content-Encoding` to `TextDecoder` fixes the issue.

---

Currently, the interceptors break on every request without me doing
anything because the `loadend` event logs the `response` which
implicitly calls `responseText()`:


https://github.com/mswjs/interceptors/blob/d8f785d2571a6941fd67e7d2f74c43db6aa528b5/src/interceptors/XMLHttpRequest/XMLHttpRequestOverride.ts#L447

The `responseText()` method passes the response' `Content-Encoding`
value to `decodeBuffer()` which passes it to `TextDecoder`.

However, it seems that the
[`Content-Encoding`](https://github.com/mswjs/interceptors/blob/main/src/interceptors/XMLHttpRequest/XMLHttpRequestOverride.ts#L483-L491)
header is conventionally for denoting the response' compression such as
`gzip`, `br`, `deflate` etc.

> Content encoding is mainly used to compress the message data without
losing information about the origin media type

See the [example
syntax](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Encoding#syntax)
and
[directives](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Encoding#directives).

Meanwhile, `TextDecoder` appears to be more concerned with [character
encoding](https://developer.mozilla.org/en-US/docs/Web/API/Encoding_API/Encodings),
such as `utf-8`.

For this reason, I recommend not passing through the `Content-Encoding`
value to `TextDecoder`, since it leads to errors on every request of
this form:

```
Uncaught RangeError: Failed to construct 'TextDecoder': The encoding label provided ('br') is invalid.
```

I confirmed that this change fixes the issue.

Once again thank you for all your great work on this project!

Co-authored-by: Artem Zakharchenko <kettanaito@gmail.com>
@kettanaito
Copy link
Member

Released: v0.19.2 🎉

This has been released in v0.19.2!

Make sure to always update to the latest version (npm i @mswjs/interceptors@latest) to get the newest features and bug fixes.


Predictable release automation by @ossjs/release.

@OlivierRiberi
Copy link
Author

Thank you for the investigation!
Unfortunately, I ran again my repro and it seems like the issue still persists with the latest version: 0.19.4
I wish I could be more helpful but I'm not familiar with the internals of @mswjs/interceptors nor got.

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

Successfully merging a pull request may close this issue.

3 participants