1986022 - Encoding of non-ASCII characters in response bodies is broken

Status: RESOLVED FIXED

(Remote Protocol :: WebDriver BiDi, defect, P3)

Description

[Holger Benl [:hbenl]]

This Playwright test fails when I try it with this PR and when I look at the raw BiDi messages I see that the character Ü is represented by the bytes C3 83 C2 9C. This looks like a double encoding (in node.js Buffer.from(Buffer.from("Ü", "utf8").toString("latin1"), "utf8") gives me <Buffer c3 83 c2 9c>).

Comment 1

[Julian Descottes [:jdescottes]]

Had a quick look, it seems we actually don't get the charset for the response, and we fail to decode the stream properly.
The channel's contentCharset is not set for this response. It seems in most spots DevTools falls back to UTF-8 when the charset is not set, so maybe we should do the same here.

Comment 2

[Julian Descottes [:jdescottes]]

Attachment: Bug 1986022 - [devtools] Default to UTF-8 when charset is missing for NetworkUtils.decodeResponseChunks

Comment 3

[Julian Descottes [:jdescottes]]

This can be entirely fixed in devtools, but I'll try to add a wdspec test for it anyway.

Comment 4

[Julian Descottes [:jdescottes]]

I wrote a wdspec test, but I actually see that Chrome has the same behavior as current Firefox so we might need a spec discussion about what to do for responses which don't define a charset.

Here maybe you could propose a PR to fix the test with

  server.setRoute('/empty.html', (req, res) => {
    res.statusCode = 200;
    res.setHeader('content-type', 'text/plain; charset=utf-8');
    res.end(Buffer.from('Ü (lowercase ü)', 'utf-8'));
  });

?

Comment 5

[Julian Descottes [:jdescottes]]

In the meantime I filed https://github.com/w3c/webdriver-bidi/issues/1005
The wdspec test is up at https://github.com/web-platform-tests/wpt/pull/54928 for discussion.

I will remove the patches from the review queue until we have an agreement.

Comment 6

[Julian Descottes [:jdescottes]]

Attachment: Bug 1986022 - [bidi] Always use UTF-8 to decode bytes for network.getData

Comment 7

[Holger Benl [:hbenl]]

Here maybe you could propose a PR to fix the test

I'd prefer to keep the test failing since it shows a real issue.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/6490a19899f5
https://hg.mozilla.org/integration/autoland/rev/e447db312c3a
[devtools] Default to UTF-8 when charset is missing for NetworkUtils.decodeResponseChunks r=bomsy,devtools-reviewers
https://github.com/mozilla-firefox/firefox/commit/ae453557a5d0
https://hg.mozilla.org/integration/autoland/rev/da770b22c2f1
[bidi] Always use UTF-8 to decode bytes for network.getData r=webdriver-reviewers,whimboo

Comment 9

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/e447db312c3a
https://hg.mozilla.org/mozilla-central/rev/da770b22c2f1