Bug 202095 - websockets handshaking broken for responses that omit status text / reason
Summary: websockets handshaking broken for responses that omit status text / reason
Status: RESOLVED CONFIGURATION CHANGED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: Safari 12
Hardware: Mac macOS 10.14
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-09-22 20:11 PDT by Joey Korkames
Modified: 2021-01-11 00:45 PST (History)
3 users (show)

See Also:


Attachments
unparsed status line passed to WebSocketChannel::fail(m_handshake->failureReason() (13.18 KB, image/png)
2019-09-22 20:11 PDT, Joey Korkames
no flags Details
inspector preview shows no parsed response (55.81 KB, image/png)
2019-09-22 20:22 PDT, Joey Korkames
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Joey Korkames 2019-09-22 20:11:22 PDT
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
Comment 1 Joey Korkames 2019-09-22 20:22:02 PDT
Created attachment 379353 [details]
inspector preview shows no parsed response
Comment 2 Radar WebKit Bug Importer 2019-09-23 10:27:24 PDT
<rdar://problem/55627726>
Comment 3 Joey Korkames 2019-09-24 10:45:52 PDT
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?
Comment 4 Joey Korkames 2021-01-09 22:16:30 PST
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.
Comment 5 youenn fablet 2021-01-11 00:44:41 PST
FWIW, WebKit Cocoa port is working on using NSURLSession WebSocket code path which seem to treat 'HTTP/1.1 101\r\n' as valid.
Comment 6 youenn fablet 2021-01-11 00:45:18 PST
Tested by modifying web socket python scripts like LayoutTests/http/tests/websocket/tests/hybi/handshake-fail-by-no-upgrade-header_wsh.py