1208114 - Firefox sends RST_STREAM for HTTPS URI stream with HTTP/2 proxy

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect)

Description

[Tatsuhiro Tsujikawa]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:40.0) Gecko/20100101 Firefox/40.0
Build ID: 20150826023504

Steps to reproduce:

Browse HTTPS site via HTTP/2 secure proxy (e.g., nghttpx) using Firefox Nightly.


Actual results:

When HTTPS URI is requested, Firefox sends CONNECT request. After getting 200 response from proxy, Firefox sends RST_STREAM with error code CANCEL.
Therefore, HTTPS page cannot be loaded anymore.


Expected results:

HTTPS URI should be loaded as before.

Comment 1

[Patrick McManus [:mcmanus]]

so this is somehow tied as a regression to 1204614 which is on firefox 43. Thanks for noting it. (flow control issue, somehow).

I'll work on it.

Comment 2

[Patrick McManus [:mcmanus]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8fe2e7457b27

Comment 3

[Patrick McManus [:mcmanus]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8bc59c41b372

Comment 4

[Patrick McManus [:mcmanus]]

Attachment: fix h2 connect tunnels

Comment 5

[u408661]

Comment on attachment 8671046 [details] [diff] [review]
fix h2 connect tunnels

Review of attachment 8671046 [details] [diff] [review]:
-----------------------------------------------------------------

::: netwerk/protocol/http/TunnelUtils.h
@@ +200,5 @@
>                           uint32_t count, uint32_t *countWritten) override final;
>    nsHttpRequestHead *RequestHead() override final;
>    void Close(nsresult reason) override final;
>  
> +  bool ConnectedReadyForInput();

I'm not wild about this name - when first reading the patch, it seemed to me like "well, if it's ready for input, why does it need to be buffered?"

Actually, I think the name itself is fine (because the stream in general is ready for input, it's just a little full right now and needs a moment), but there might need to be a clarifying comment that the whole "ready for input" part is more general, but the stream could still return WOULD_BLOCK.

Comment 6

[Patrick McManus [:mcmanus]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3cd86929d8835f2b162dc2c8654ae9af2a54e06f
bug 1208114 - fix h2 connect tunnels r=hurley

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/0b1fbd9ec549 for Mediatest crashes:

https://treeherder.mozilla.org/logviewer.html#?job_id=15406183&repo=mozilla-inbound

Comment 8

[Patrick McManus [:mcmanus]]

so the mediatest fail was in my try run - sorry about that. 

This is h2 only code, so I only actually looked at the xpcshell results as that's the only server in our suite that knows h2.

turns out the mediatest is contacting youtube (h2). I thought offsite tests were something we didn't do..

Comment 9

[Patrick McManus [:mcmanus]]

The new assert included in this patch set did indeed catch a bug - but its a latent unrelated one. I'll reland without the assert (it will have the same old legacy runtime behavior) and file a separate bug about the new issue.

Http2Session::OnWriteSegment has a state machine and for any state not covered in there it returns NS_ERROR_UNEXPECTED. I added the assert() when that happens.

The state generated in https://www.youtube.com/watch?v=1visYpIREUM was DISCARDING_DATA_FRAME_PADDING which is probably ok to whitelist as a nop - but lets do it separately.

Comment 10

[Patrick McManus [:mcmanus]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c66bf1d6890b605846f5a3dd49d7e1aad4563bbb
bug 1208114 - fix h2 connect tunnels r=hurley

Comment 11

[Carsten Book [:Tomcat]]

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

Comment 12

[Tatsuhiro Tsujikawa]

I confirmed this bug has been fixed in current nightly.
Thank you very much!