1994996 - Handle unicode headers for tentative setExtraHeaders test

Status: RESOLVED FIXED

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

Description

[Julian Descottes [:jdescottes]]

At the moment we only fail two test cases from the setExtraHeaders test suite, when setting a unicode header '你好世界'

Comment 1

[Julian Descottes [:jdescottes]]

Attachment: Bug 1994996 - [necko] Change header value parameters from ACString to AUTF8String for proper UTF-8 support

Comment 2

[Julian Descottes [:jdescottes]]

Hi Kershaw,

In BiDi we are using nsIHttpChannel.setRequestHeader to modify request headers, and there is a webplatform test which asserts that we can set a header with the value 你好世界. But that doesn't seem to work with setRequestHeader (see attached test), the value stored is ``}\u0016L`.

At this point I'm not sure if this a bug in setRequestHeader, or if I should encode the header value before calling setRequestHeader, or if this value is simply not acceptable for a header? Let me know what you think!

Comment 3

[Julian Descottes [:jdescottes]]

(unassigning for now)

Comment 4

[Kershaw Chang [:kershaw]]

(In reply to Julian Descottes [:jdescottes] from comment #2)

Hi Kershaw,

In BiDi we are using nsIHttpChannel.setRequestHeader to modify request headers, and there is a webplatform test which asserts that we can set a header with the value 你好世界. But that doesn't seem to work with setRequestHeader (see attached test), the value stored is ``}\u0016L`.

At this point I'm not sure if this a bug in setRequestHeader, or if I should encode the header value before calling setRequestHeader, or if this value is simply not acceptable for a header? Let me know what you think!

The issue is in how XPConnect converts a JavaScript string to ACString (the IDL type of setRequestHeader's value parameter). ACString is a raw byte string with no encoding. When XPConnect converts a JS string to ACString, it calls JS_EncodeStringToBuffer which truncates each UTF-16 code unit to its low byte (buffer[i] = char(src[i])). So "你好世界" (U+4F60, U+597D, U+4E16, U+754C) becomes the 4 garbage bytes `}\x16L — just the low byte of each code point.

On the HTTP spec side, RFC 9110 §5.5 allows bytes in the range %x80-FF in header values, and only forbids CR/LF/NUL.
I assume what the WPT expects is the UTF-8 encoding of "你好世界" as raw bytes?
If so, I think the fix can be adding another API to use AUTF8String instead of ACString for the value parameter.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/c8fcc7daeb65
https://hg.mozilla.org/integration/autoland/rev/2afe0d717f4d
[necko] Change header value parameters from ACString to AUTF8String for proper UTF-8 support r=necko-reviewers,Sasha,kershaw

Comment 6

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/2afe0d717f4d

Comment 7

[Julian Descottes [:jdescottes]]

Reopening the bug since Bug 2020207 will revert the fix. We need to find a solution for this which doesn't regress other consumers setting reading headers.

Comment 8

[Julian Descottes [:jdescottes]]

Attachment: Bug 1994996 - [bidi] Encode/Decode protocol bytes as UTF8 in the network module

Comment 9

[Julian Descottes [:jdescottes]]

Attachment: Bug 1994996 - [wdspec] Add tests for non-utf8 headers with various network commands

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/c6d98f856c2f
https://hg.mozilla.org/integration/autoland/rev/20457722a6da
[bidi] Encode/Decode protocol bytes as UTF8 in the network module r=Sasha,valentin

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/6ab3366d3240
https://hg.mozilla.org/integration/autoland/rev/abddc05aaa2d
[wdspec] Add tests for non-utf8 headers with various network commands r=Sasha

Comment 12

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/20457722a6da

Comment 13

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/abddc05aaa2d

Comment 14

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/58310 for changes under testing/web-platform/tests

Comment 15

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot