All users were logged out of Bugzilla on October 13th, 2018

h2 alt-svc extension update to -06

RESOLVED FIXED in Firefox 38

Status

()

RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: mcmanus, Assigned: mcmanus)

Tracking

unspecified
mozilla38
x86
Windows 7
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox38 fixed)

Details

Attachments

(3 attachments, 1 obsolete attachment)

(Assignee)

Description

4 years ago
changes from 04 to 06

* alt-used contents
* h2 frame format

also added a test for alt-svc frame on stream 0

also added node-http2 support for generating -06 compatible altsvc frames to enable the test. This isn't really suitable for upstream as it doesn't do frame reciept or the node-http2 internal framing tests correctly. But it should be fine as a placeholder in our CI.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ff8bdd58e8d5
(Assignee)

Comment 1

4 years ago
Created attachment 8561067 [details]
MozReview Request: bz://1130874/mcmanus

/r/3543 - bug 1130874 - update h2 alternate service extension to draft-06 r=hurley
/r/3545 - bug 1130874 - h2 test server altsvc 06 r=hurley
/r/3547 - bug 1130874 - test for alt-svc -06 h2 extension r=hurley

Pull down these commits:

hg pull review -r 2f4920057e25f1b60864ab573fc6b6bad1169563
(Assignee)

Updated

4 years ago
Attachment #8561067 - Flags: review?(hurley)
https://reviewboard.mozilla.org/r/3547/#review2829

Looks pretty good to me. Just one comment that I would like clarification on, and some fist-shaking at your editor :)

FYI, I filed https://github.com/molnarg/node-http2/issues/95 to update node-http2 appropriately (we can update CI to use the new full version whenever I get to writing those bits).

::: netwerk/test/unit/test_http2.js
(Diff revision 1)
> +            , test_http2_continuations

Why was this moved?

::: testing/xpcshell/moz-http2/moz-http2.js
(Diff revision 1)
> +	req.headers['alt-used'] != ("localhost:" + serverPort)) {

... and here ...

::: testing/xpcshell/moz-http2/moz-http2.js
(Diff revision 1)
> +	req.scheme != "http" ||

... and here ...

::: testing/xpcshell/moz-http2/moz-http2.js
(Diff revision 1)
> +	req.headers['alt-used'] != ("localhost:" + serverPort)) {

... and here.

::: testing/xpcshell/moz-http2/moz-http2.js
(Diff revision 1)
> +	req.scheme != "http" ||

Your editor once again did leading tabs instead of spaces here...
Comment on attachment 8561067 [details]
MozReview Request: bz://1130874/mcmanus

Awwww, reviewboard posted the comments on moz-http2.js in reverse order, totally ruining my extended review sentence :(

It also didn't set r+ for some reason beyond understanding, even though I said "ship it!".
Attachment #8561067 - Flags: review?(hurley) → review+
(Assignee)

Comment 4

4 years ago
> 
> ::: netwerk/test/unit/test_http2.js
> (Diff revision 1)
> > +            , test_http2_continuations
> 
> Why was this moved?
> 

    when running the test locally I would periodically see this test fail on a node-http2 exception that "res didn't have a method push".. and it was apparent that the transaction was using h1. It think perhaps the wrongsuite test occasionally managed to make an extra connection (via preconnect?) that negotiated down to h1. Anyhow, moving the test up to the point in the manifest where there is less h1 traffic resolved it.
Oh, joy. Well, WFM, then :)
https://hg.mozilla.org/mozilla-central/rev/f0e2f7d00b82
https://hg.mozilla.org/mozilla-central/rev/eb0c935a8fda
https://hg.mozilla.org/mozilla-central/rev/717a6673bc18
Status: NEW → RESOLVED
Last Resolved: 4 years ago
status-firefox38: --- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
(Assignee)

Comment 8

3 years ago
Comment on attachment 8561067 [details]
MozReview Request: bz://1130874/mcmanus
Attachment #8561067 - Attachment is obsolete: true
Attachment #8619406 - Flags: review+
Attachment #8619407 - Flags: review+
Attachment #8619408 - Flags: review+
(Assignee)

Comment 9

3 years ago
Created attachment 8619406 [details]
MozReview Request: bug 1130874 - h2 test server altsvc 06 r=hurley
(Assignee)

Comment 10

3 years ago
Created attachment 8619407 [details]
MozReview Request: bug 1130874 - test for alt-svc -06 h2 extension r=hurley
(Assignee)

Comment 11

3 years ago
Created attachment 8619408 [details]
MozReview Request: bug 1130874 - update h2 alternate service extension to draft-06 r=hurley
You need to log in before you can comment on or make changes to this bug.