1772213 - Refactor the WebSockets over HTTP/2 code

Status: RESOLVED FIXED

(Core :: Networking: WebSockets, task, P3)

Description

[Dragana Damjanovic [:dragana]]

Current state:

The current code does not work if WebSockets over HTTP/2 are used over a HTTP/2 proxy.
Additional complexity is that the CONNECT request is driven in 2 ways at the moment:

• In the case of HTTP/1.1 proxy: nsHttpConnection is responsible for driving the CONNECT request
• In the case of HTTP/2 proxy: Http2ConnectTransaction drives the CONNECT request.

The proxy code will be refactored and implies that the WebSocket code needs to be refactored as well, because the 2 feature use the same code.

Proposal

When a new nsHttpTransaction for WebSocket needs to be dispatched to a HTTP/2 connection, the following steps will be performed:

• Create Http2StreamConnention (see 1772205)
• Create a nsHttpConnection object for the connection through the proxy and register it to the nsHttpConnectionMgr. The nsHttpConnection will use Http2StreamConnention as nsISocketTransport and Http2StreamConnectionOutputStream and Http2StreamConnectionInputStream as nsSocketInoput/OutputStream.
• Dispatch the transaction to the nsHttpConnection.
• The nsHttpConnection will be responsible for driving CONNECT request.

Comment 1

[Dragana Damjanovic [:dragana]]

Note: Http2StreamConnention from above change the name to Http2StreamTunnel.

Comment 2

[Dragana Damjanovic [:dragana]]

After refactoring work for the proxy code, WebSocket over HTTP/2 was changed as well. Now it uses Http2StreamWebsocket a special version of Http2StreamBase. The code still uses Http2ConnectTransaction that acts as a transaction to Http2StreamWebSocket, it also holds the nsHttpTransaction created for the WebSocket and calls rnsHttpTransaction's eads and writes as appropriate.

The new architecture

We will use Http2StreamTunnel to act a stream to Http2Session and as a fake nsISocketTransport to nsHttpConnection.
The difference between WebSocket over HTTP/2 and WebSockets love HTTP/1.1 will be:

HTTP/1.1 case:

• WebSocket -> nsHttpChannel -> nsHttpTransaction -> nsHttpConnection -> nsSocketTransport
• nsHttpConnection hold reference to nsSocketTransport in mTransport and reference to nsSocketInputStream and nsSocketOutputStream in mSocketIin, mSocketOut.

HTTP/2 case should be very similar:

• WebSocket -> nsHttpChannel -> nsHttpTransaction -> nsHttpConnection -> Http2StreamTunnel
• nsHttpConnection hold reference to nsSocketTransport in mTransport and reference to InputStreamTunnel and OutputStreamTunnel in mSocketIin, mSocketOut.
• Nothing should change for WebSocket, nsHttpChannel, nsHttpTransaction and nsHttpConnection and only Http2StreamTunnel will replace nsSocketTransport.

Dispatching nsHttpTransaction that serves a WebSocket request

HTTP/2 support for WebSocket needs to be negotiated via HTTP/2 setting. Therefore when a HTTP/2 session is created we do not know if WebSockets are supported until the settings from the server are received.

Current dispatching is happening here. There are a couple of steps and I will only explain the most import steps for this particular work:

• here we look for an existing HTTP/2 or HTTP/3 connection (note: for WebSocket HTTP/3 will be disabled via NS_HTTP_DISALLOW_HTTP3 flag so we will not look for a HTTP/3 connection only HTTP/2)
  There are these outcomes:
  • We have not found an HTTP/2 session, process to create a new connection (this connection may be HTTP/2 or HTTP/1.1 we do not know at this moment)
  • We have found an HTTP/2 connection and the server does not support WebSocket. We will disable HTTP/2 for the transaction and process to create a HTTP/1.1 connection. This code
  • We have found an HTTP/2 connection that supports WebSocket or we have found an HTTP/2 connection that has not received a setting from the server yet and we do not know if WebSocket is supported or not. In both cases the nsHttpTrannsaction will be added to HTTP/2 session(this code and this code) but treated a bit differently:
    • If this was an HTTP/2 connection that supports WebSocket, Http2StreamWebsocket will be created immediatelly
    • if not the transaction will be queued inside Http2Session.

What we need to change

The Http2StreamTunnel for the proxy code is created here. What that does is:

• we have an HTTP/2 connection to a proxy and we need to create a tunnel for a new request through the proxy.
• CreateTunnelStream() creates a Http2StreamTunnel and a nsHttpConnection. The connection will use the Http2StreamTunnel as its fake nsISocketTransport.
• We add the new connection to the connection entry register (this code)
• Call SetInSpdyTunnel to tell the new connection that it is inside a tunnel so another TLS handshake is not needed
• and call DispatchTransaction to dispatch the transaction to the new connection.

We will need to do the same for WebSockets:

• instead of calling DispatchTransaction here, in case transaction is for websocket(trans->IsWebsocketUpgrade()) we will create a new Http2StreamTunnel (this). For non-WebSocket transaction nothing should change.

CanAcceptWebsocket to return true if the server supports WebSocket or if we still do not know. We will need to distinguish these 2 cases and CanAcceptWebsocket cannot return true but 3 possible outcomes: supports, does-not-support, we-do-not-know:

• In the does-not-support case: will not change the behavior, leave it the same as above
• In the supports case: create a new Http2StreamTunnel
• In the we-do-not-know case: return from the function return NS_ERROR_NOT_AVAILABLE. This will queue the transaction.

What to do with queued transactions:
When HTTP/2 settings from the server are received and we know whether WebSoket are supported or not call gHttpHandler->ConnMgr()->ProcessPendingQ(mConnInfo); That will retriger processing of the queued transactons.

What to do with a transaction that was used to drive TLS handshake:
nsHttpTransaction is used for driving TLS handshake. When we choose a nsHttpTransaction for this we do not know if the server will use HTTP/2. So we will have a strange situation when we found out that the server uses HTTP/2 (here and here) and nsHttpTransaction is for a webSocket, because we do not know whether the server supports WebSockets yet. We will need to restart the transaction in this case. That will return the transaction to the nsHttpConnectionMgr and transaction will be dispatched again. The restart of nsHttpTransaction is performed like this. The best place to do the restart is here

Some notes: We have a mechanism in nsHttpConnectionMgr to make sure that we only have one active HTTP/2 session(that is not in the process of shutting down(GOAWAY)) to a host. Connections to webSocket over HTTP/1.1 are exceptions because if a HTTP/2 server does not support WebSocket we can have multiple connections, i.e. one HTTP/2 for normal request and one or more HTTP/1 for WebSockets. We may need to add additional code so that our connections do not get closed! We will see, I expect that it should just work, but warning in advance.

Comment 3

[Dragana Damjanovic [:dragana]]

There are a couple of additional things that need to change.

The Generating headers code needs to be updated in Http2StreamTunnel. It needs to be more generic to accommodate WebSockets and proxy CONNECT. The headers for Http2StreamTunnel are generated here. Even further, I think we do not need a special version for Http2Stream and Http2StreamTunnel. I think mFlatHttpRequestHeaders has enough information so that session->Compressor()->EncodeHeaderBlock(..) can distinguish between proxy, websocket and normal request.

nsHttpConnection::HandleWebSocketResponse needs to be updated to handle webSocket response from HTTP/2 which is 200 instead of 101 for HTTP/1.1.

This special casing for websocket should not be needed anymore.

There will be some cleaning necessary as well, e.g. removing Http2ConnectTransaction, Http2StreamWebSocket, removing queuing code for transactions in Http2Session, etc.

Comment 4

[edgul]

Attachment: Bug 1772213 - Refactoring websockets to use Http2StreamTunnel for websocket over http2 (may not provide support for websocket over proxy). r=kershaw,dragana

Comment 5

[edgul]

Attachment: Bug 1772213 - Refactored websockets to use Http2StreamTunnel for websocket over http2. r=kershaw,valentin

Comment 6

[Kershaw Chang [:kershaw]]

Just to share the observation about this test failure.
The http log shows shat we've received the 401 Unauthorized response.

 0:28.11 pid:53633 [Parent 53633: Socket Thread]: V/nsHttp nsHttpTransaction::HandleContentStart [this=2ca0b9800]
 0:28.11 pid:53633 [Parent 53633: Socket Thread]: I/nsHttp http response [
 0:28.11 pid:53633 [Parent 53633: Socket Thread]: E/nsHttp   HTTP/2 401 Unauthorized
 0:28.11 pid:53633 [Parent 53633: Socket Thread]: E/nsHttp   content-length: 0
 0:28.11 pid:53633 [Parent 53633: Socket Thread]: E/nsHttp   www-authenticate: Basic realm="camelot"
 0:28.11 pid:53633 [Parent 53633: Socket Thread]: E/nsHttp   X-Firefox-Spdy: h2
 0:28.11 pid:53633 [Parent 53633: Socket Thread]: E/nsHttp     OriginalHeaders
 0:28.11 pid:53633 [Parent 53633: Socket Thread]: E/nsHttp   content-length: 0
 0:28.11 pid:53633 [Parent 53633: Socket Thread]: E/nsHttp   www-authenticate: Basic realm="camelot"
 0:28.11 pid:53633 [Parent 53633: Socket Thread]: E/nsHttp   X-Firefox-Spdy: h2
 0:28.11 pid:53633 [Parent 53633: Socket Thread]: I/nsHttp ]
 0:28.11 pid:53633 [Parent 53633: Socket Thread]: V/nsHttp nsHttpTransaction::CheckForStickyAuthScheme this=2ca0b9800
 0:28.11 pid:53633 [Parent 53633: Socket Thread]: V/nsHttp   already sticky
 0:28.11 pid:53633 [Parent 53633: Socket Thread]: V/nsHttp   already sticky
 0:28.11 pid:53633 [Parent 53633: Socket Thread]: V/nsHttp nsHttpConnection::OnHeadersAvailable [this=2cbeddc00 trans=2ca0b9800 response-head=2ca8d5760]
 0:28.11 pid:53633 [Parent 53633: Socket Thread]: V/nsHttp Connection can be reused [this=2cbeddc00 idle-timeout=115sec]

Than, we try to do the auth retry with the sticky connection.

 0:28.11 pid:53633 [Parent 53633: Main Thread]: E/nsHttp nsHttpTransaction::Init [this=2ca0b9300 caps=120a104]
 0:28.11 pid:53633 [Parent 53633: Main Thread]: E/nsHttp nsHttpTransaction 2ca0b9300 SetRequestContext 0
 0:28.11 pid:53633 [Parent 53633: Main Thread]: E/nsHttp http request [
 0:28.11 pid:53633 [Parent 53633: Main Thread]: E/nsHttp   GET /basic_auth HTTP/1.1
 0:28.11 pid:53633 [Parent 53633: Main Thread]: E/nsHttp   Host: web-platform.test:9000
 0:28.11 pid:53633 [Parent 53633: Main Thread]: E/nsHttp   User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:108.0) Gecko/20100101 Firefox/108.0
 0:28.11 pid:53633 [Parent 53633: Main Thread]: E/nsHttp   Accept: */*
 0:28.11 pid:53633 [Parent 53633: Main Thread]: E/nsHttp   Accept-Language: en-US,en;q=0.5
 0:28.11 pid:53633 [Parent 53633: Main Thread]: E/nsHttp   Accept-Encoding: gzip, deflate, br
 0:28.11 pid:53633 [Parent 53633: Main Thread]: E/nsHttp   Sec-WebSocket-Version: 13
 0:28.11 pid:53633 [Parent 53633: Main Thread]: E/nsHttp   Origin: https://web-platform.test:9000
 0:28.11 pid:53633 [Parent 53633: Main Thread]: E/nsHttp   Sec-WebSocket-Extensions: permessage-deflate
 0:28.11 pid:53633 [Parent 53633: Main Thread]: E/nsHttp   Sec-WebSocket-Key: w4Bul4MT+3nxcENTsOXflg==
 0:28.11 pid:53633 [Parent 53633: Main Thread]: E/nsHttp   Connection: keep-alive, Upgrade
 0:28.11 pid:53633 [Parent 53633: Main Thread]: E/nsHttp   Sec-Fetch-Dest: websocket
 0:28.11 pid:53633 [Parent 53633: Main Thread]: E/nsHttp   Sec-Fetch-Mode: websocket
 0:28.11 pid:53633 [Parent 53633: Main Thread]: E/nsHttp   Sec-Fetch-Site: same-origin
 0:28.11 pid:53633 [Parent 53633: Main Thread]: E/nsHttp   Pragma: no-cache
 0:28.11 pid:53633 [Parent 53633: Main Thread]: E/nsHttp   Cache-Control: no-cache
 0:28.11 pid:53633 [Parent 53633: Main Thread]: E/nsHttp   Upgrade: websocket
 0:28.11 pid:53633 [Parent 53633: Main Thread]: E/nsHttp   Authorization: ******************
 0:28.11 pid:53633 [Parent 53633: Main Thread]: E/nsHttp
 0:28.11 pid:53633 [Parent 53633: Main Thread]: E/nsHttp ]
 0:28.11 pid:53633 [Parent 53633: Main Thread]: V/nsHttp nsHttpConnectionMgr::AddTransactionWithStickyConn [trans=2ca0b9310 0 transWithStickyConn=2ca0b9810]
 0:28.11 pid:53633 [Parent 53633: Main Thread]: D/nsHttp nsHttpChannel::ContinueOnStopRequestAfterAuthRetry [this=2ca03e000, aStatus=0 aAuthRetry=1, aIsFromNet=1, aTransWithStickyConn=2ca0b9810]
 0:28.11 pid:53633 [Parent 53633: Socket Thread]: V/nsHttp nsHttpConnectionMgr::OnMsgNewTransactionWithStickyConn [trans=2ca0b9300, transWithStickyConn=2ca0b9800, conn=2cbee1ae0]
 0:28.11 pid:53633 [Parent 53633: Socket Thread]: V/nsHttp  Reuse connection [2cbee1ae0] for transaction [2ca0b9300]
 0:28.11 pid:53633 [Parent 53633: Socket Thread]: V/nsHttp FindCoalescableConnection .S........[tlsflags0x00000000]web-platform.test:9000[foo]^partitionKey=%28https%2Cweb-platform.test%29
 0:28.11 pid:53633 [Parent 53633: Socket Thread]: E/nsHttp TlsHandshaker::SetupSSL 2ca2f3a00 caps=0x121 .S........[tlsflags0x00000000]web-platform.test:9000^partitionKey=%28https%2Cweb-platform.test%29
 0:28.11 pid:53633 [Parent 53633: Socket Thread]: V/nsHttp FindCoalescableConnectionByHashKey() found match conn=2ca2f3a00 key=127.0.0.1~.:~.:9000/[^partitionKey=%28https%2Cweb-platform.test%29]viaDNS newCI=.S........[tlsflags0x00000000]web-platform.test:9000[foo]^partitionKey=%28https%2Cweb-platform.test%29 matchedCI=.S........[tlsflags0x00000000]web-platform.test:9000^partitionKey=%28https%2Cweb-platform.test%29 join ok
 0:28.11 pid:53633 [Parent 53633: Socket Thread]: V/nsHttp FindCoalescableConnection(.S........[tlsflags0x00000000]web-platform.test:9000[foo]^partitionKey=%28https%2Cweb-platform.test%29) match conn 2ca2f3a00 on dns key 127.0.0.1~.:~.:9000/[^partitionKey=%28https%2Cweb-platform.test%29]viaDNS
 0:28.11 pid:53633 [Parent 53633: Socket Thread]: V/nsHttp GetH2orH3ActiveConn() request for ent 1374f1bb0 .S........[tlsflags0x00000000]web-platform.test:9000[foo]^partitionKey=%28https%2Cweb-platform.test%29 found an active connection 2ca2f3a00 in the coalescing hashtable
 0:28.11 pid:53633 [Parent 53633: Socket Thread]: V/nsHttp nsHttpConnectionMgr::ProcessNewTransaction trans=2ca0b9300 sticky connection=2cbeddc00

In the end, the test server just doesn't response and the test is timeout.

It seems the problem is about whether we can reuse the same connection. I've tried to not reuse this connection and this test was passed for me locally.

Comment 7

[edgul]

Attachment: Bug 1772213 - Fixes for http2 websocket wpt failures r=valentin

Depends on D159541

Comment 8

[edgul]

Post-merge (wpt fixes) try auto looks good:
https://treeherder.mozilla.org/jobs?repo=try&revision=f08103f15f67e681c407b8d51ce00f15d5e7328f
Post-merge (wpt fixes) fuzzy wpt: has macOS issues, but these are pre-existing (see below):
https://treeherder.mozilla.org/jobs?repo=try&revision=cbce790f9eb63e0c359af6e012b558528ae46f5b&selectedTaskRun=GhmpV8Y3QOOdZburaMTYoQ.0
Central fuzzy wpt, we see macOS issues here:
https://treeherder.mozilla.org/jobs?repo=try&revision=5beb3ce2e1405019ad5e7987870e4b440dc27051
Pre-merge (wpt fixes) more thorough web-platform tests:
https://treeherder.mozilla.org/jobs?repo=try&revision=3e9f2027c7879c236f7d3b4b7a246fc962128c21&group_state=expanded
Local xpcshell-tests websocket tests confirmed as well:
network/test/unit/test_websocket_*
network/test/unit/test_http2-proxy*

Just need reviews to attempt to land now:
https://phabricator.services.mozilla.com/D159541
https://phabricator.services.mozilla.com/D163254

Comment 9

[Valentin Gosu [:valentin] (he/him)]

Thanks!

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2e430bd2c9f7
Refactored websockets to use Http2StreamTunnel for websocket over http2. r=kershaw,necko-reviewers

Comment 11

[Natalia Csoregi [:nataliaCs]]

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