1971997 - HTTP/2 and HTTP/3 should not reparse the headers.

Status: ASSIGNED

(Core :: Networking: HTTP, enhancement, P2)

Description

[Valentin Gosu [:valentin]]

As can be noticed in bug 1965246, we have an odd way of passing headers from the transaction to the H2 and H3 stream.

When opening the channel and creating the transaction, we'll serialize the requestHead in mozilla::net::nsHttpTransaction::Init on the main thread.
Then we create an input stream that contains the headers and possibly body.
When the transaction gets dispatched we call:
mozilla::net::nsHttpTransaction::ReadSegments
-> mozilla::net::nsAHttpSegmentReader::OnReadSegment which reads from the stream.
The headers then get read into a different string for H2 in mozilla::net::Http2StreamBase::OnReadSegment then passed to EncodeHeaderBlock in mozilla::net::Http2Stream::GenerateHeaders which parses the string and compresses the headers with HPACK mozilla::net::Http2Compressor::EncodeHeaderBlock.

Similarly for H3 mFlatHttpRequestHeaders gets passed into neqo that again parses it into a header array in neqo_http3conn_fetch

Just passing in a nsHttpRequestHead* to HPACK and neqo_http3conn_fetch would be much better - it would avoid the allocations and reparsing.

Comment 1

[Valentin Gosu [:valentin]]

Attachment: Bug 1971997 - Remove uploadStreamHasHeaders from nsIUploadChannel2 r=#necko

Comment 2

[Valentin Gosu [:valentin]]

Attachment: Bug 1971997 - Make Request head const r=#necko

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/ae6a9de29bdf
https://hg.mozilla.org/integration/autoland/rev/9c45995e2912
HTTP/2 and HTTP/3 should not reparse the headers. r=necko-reviewers,geckoview-reviewers,extension-reviewers,devtools-reviewers,jesup,robwu,nalexander,bomsy
https://github.com/mozilla-firefox/firefox/commit/fc0985269088
https://hg.mozilla.org/integration/autoland/rev/cbd35d9c831d
Make Request head const r=necko-reviewers,jesup

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/e8aa1cc58d36
https://hg.mozilla.org/integration/autoland/rev/5efb7b69f730
Revert "Bug 1971997 - Make Request head const r=necko-reviewers,jesupi" for causing causing mocitest http3 failures @ test_XHRSendData.html

Comment 5

[pstanciu]

Backed out for causing mocitest http3 failures @ test_XHRSendData.html

• https://hg.mozilla.org/integration/autoland/rev/5efb7b69f7304c7e448836c37d0f1c51e6201951
• Push with failures
• Failure Log test_XHRSendData.html
• Failure Log mochitest/test_bug_776171.html
• Failure Log mochitest/test_xhr_method_case.html
• Failure Log test_bug444722.html
• Push with failures | WPT T2
• Failure Log | WPT T2

Comment 6

[Valentin Gosu [:valentin]]

Attachment: Bug 1971997 - Remove uploadStreamHasHeaders from nsIUploadChannel2 r=#necko

Comment 7

[Valentin Gosu [:valentin]]

Attachment: Bug 1971997 - Make Request head const r=#necko

Comment 8

[Valentin Gosu [:valentin]]

Attachment: Bug 1971997 - HTTP/2 and HTTP/3 should not reparse the headers. r=#necko

Comment 9

[Valentin Gosu [:valentin]]

Attachment: Bug 1971997 - Fix happy eyeballs r=#necko

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/664d1913d5e2
https://hg.mozilla.org/integration/autoland/rev/5ce10b2d67d1
Remove uploadStreamHasHeaders from nsIUploadChannel2 r=necko-reviewers,geckoview-reviewers,extension-reviewers,devtools-reviewers,jesup,robwu,nalexander,bomsy,media-playback-reviewers,alwu
https://github.com/mozilla-firefox/firefox/commit/61bde9c80a4d
https://hg.mozilla.org/integration/autoland/rev/2bf8ee9788d8
Make Request head const r=necko-reviewers,jesup
https://github.com/mozilla-firefox/firefox/commit/e0dffbb8ef61
https://hg.mozilla.org/integration/autoland/rev/6647b15c7470
apply code formatting via Lando

Comment 11

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/5ce10b2d67d1
https://hg.mozilla.org/mozilla-central/rev/2bf8ee9788d8
https://hg.mozilla.org/mozilla-central/rev/6647b15c7470