Bug 202095 – websockets handshaking broken for responses that omit status text / reason

• Status: RESOLVED
• Resolution: CONFIGURATION CHANGED
• Component: WebCore Misc.

[Joey Korkames]

Created attachment 379352 [details]
unparsed status line passed to WebSocketChannel::fail(m_handshake->failureReason()

websocket servers that respond with truncated "h2 style" status lines like 'HTTP/1.1 101\r\n' are rejected by Safari:

$ curl -si 'https://web.voice.telephony.goog/websocket' -H 'Host: web.voice.telephony.goog' -H 'Upgrade: websocket' -H 'Connection: Upgrade' -H "Sec-WebSocket-Key: $(openssl rand -base64 16)" -H 'Origin: https://voice.google.com' -H 'Sec-WebSocket-Protocol: sip' -H 'Sec-WebSocket-Version: 13' | head -n1 | hexdump -c
0000000   H   T   T   P   /   1   .   1       1   0   1  \r  \n
000000e

I've cross reported this to the Google Voice forum:
https://support.google.com/voice/thread/14998073?msgid=14998073

That endpoint is for a product that specifically detects-and-degrades on Safari, but maybe that is not such an unusual kind of response line.

Chromium and Firefox use their canonical browsing parsers to validate the handshake's response, and theirs do not
seem to require a status text:
https://chromium.googlesource.com/chromium/chromium/+/fe2e997ac2990d3d5b9c8abe653776830aaf8c28/net/http/http_response_headers.cc#728
https://searchfox.org/mozilla-central/source/netwerk/protocol/http/nsHttpResponseHead.cpp#506

whereas webkit requires the status-code sent between two spaces:
https://trac.webkit.org/browser/webkit/trunk/Source/WebCore/Modules/websockets/WebSocketHandshake.cpp?rev=249013#L453

[Joey Korkames]

Created attachment 379353 [details]
inspector preview shows no parsed response

[Radar WebKit Bug Importer]

<rdar://problem/55627726>

[Joey Korkames]

autobahn.py's WS testsuite* seems to agree with Chrome & FF handshake impls, but not webkit's:
  # Response Line
  #
  sl = self.http_status_line.split()
  if len(sl) < 2:
     return self.failHandshake("Bad HTTP response status line '%s'" % self.http_status_line)

In [1]: len("HTTP/1.1 101".split())
Out[1]: 2

1: https://github.com/crossbario/autobahn-python/blob/f0d15f02735429e3f92ea56502b79b33acc65882/autobahn/websocket/protocol.py#L3674)


Its tyranny of the majority (of UAs), including Safari, if its using NSURLSession & I'm following it accurately:

https://opensource.apple.com/source/CFNetwork/CFNetwork-129.20/HTTP/CFHTTPStream.c.auto.html
httpReceiveResponse is just checking it got all the bytes it can get


nextActionForHeaders() calls CFHTTPMessageGetResponseStatusCode(CFMessageRef headers)...

https://opensource.apple.com/source/CFNetwork/CFNetwork-129.20/HTTP/CFHTTPMessage.c.auto.html

looks for the .flags[status] that *_extractResponseStatusLine got when initializing the CFHTTPMessage (after _parseHeadersFromData was fired on the last append of message's bytes)

*_extractResponseStatusLine(...) seems to parse for just the code numbers

Very hard to follow!

But this is borne out when trying to browse this server in Safari.

it's inspector says:
"Failed to load resource: the server responded with a status of 400 () -- https://web.voice.telephony.goog/favicon.ico"

showing CFNetwork does tolerate the truncated status line.

I suppose there's already a task open to convert webcore:websockets to NSURLSession or CFURLConnection?

[Joey Korkames]

The site in the initial report has fixed their status line to be HTTP1.1 conformant.

I don't have another candidate to test against.

[youenn fablet]

FWIW, WebKit Cocoa port is working on using NSURLSession WebSocket code path which seem to treat 'HTTP/1.1 101\r\n' as valid.

[youenn fablet]

Tested by modifying web socket python scripts like LayoutTests/http/tests/websocket/tests/hybi/handshake-fail-by-no-upgrade-header_wsh.py