Commit 1a9f003e authored by Bence Béky's avatar Bence Béky Committed by Commit Bot

Disable WebSockets over HTTP/2 support to avoid crash.

This crash was introduced in https://crrev.com/c/940463.  It seems to be
triggered by buggy servers that negotiate HTTP/2 and send 200 OK to an
HTTP/1.1 WebSocket request.  This CL reverts the functional part of that
change and in effect disables WebSocket over HTTP/2 support.  Behavior
is not spec-compliant if Chrome is invoked with the experimental
--enable-websocket-over-http2 command line flag.  In particular, crash
might still happen when using this flag.

Bug: 842575
Change-Id: I8ebb60cda3e056ad37557bd1cfc92088224b80a1
Reviewed-on: https://chromium-review.googlesource.com/1067986Reviewed-by: default avatarAdam Rice <ricea@chromium.org>
Commit-Queue: Bence Béky <bnc@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560454}
parent 9eef34b6
...@@ -323,16 +323,7 @@ void Delegate::OnResponseStarted(URLRequest* request, int net_error) { ...@@ -323,16 +323,7 @@ void Delegate::OnResponseStarted(URLRequest* request, int net_error) {
const int response_code = request->GetResponseCode(); const int response_code = request->GetResponseCode();
DVLOG(3) << "OnResponseStarted (response code " << response_code << ")"; DVLOG(3) << "OnResponseStarted (response code " << response_code << ")";
if (request->response_info().connection_info == // TODO(https://crbug.com/842575) Re-implement WebSocket over HTTP/2.
HttpResponseInfo::CONNECTION_INFO_HTTP2) {
if (response_code == HTTP_OK) {
owner_->PerformUpgrade();
return;
}
owner_->ReportFailure(net_error);
return;
}
switch (response_code) { switch (response_code) {
case HTTP_SWITCHING_PROTOCOLS: case HTTP_SWITCHING_PROTOCOLS:
......
...@@ -386,9 +386,10 @@ INSTANTIATE_TEST_CASE_P(, ...@@ -386,9 +386,10 @@ INSTANTIATE_TEST_CASE_P(,
using WebSocketMultiProtocolStreamCreateTest = WebSocketStreamCreateTest; using WebSocketMultiProtocolStreamCreateTest = WebSocketStreamCreateTest;
// TODO(https://crbug.com/842575) Re-enable testing with HTTP2_HANDSHAKE_STREAM.
INSTANTIATE_TEST_CASE_P(, INSTANTIATE_TEST_CASE_P(,
WebSocketMultiProtocolStreamCreateTest, WebSocketMultiProtocolStreamCreateTest,
Values(BASIC_HANDSHAKE_STREAM, HTTP2_HANDSHAKE_STREAM)); Values(BASIC_HANDSHAKE_STREAM));
// There are enough tests of the Sec-WebSocket-Extensions header that they // There are enough tests of the Sec-WebSocket-Extensions header that they
// deserve their own test fixture. // deserve their own test fixture.
...@@ -408,9 +409,10 @@ class WebSocketStreamCreateExtensionTest ...@@ -408,9 +409,10 @@ class WebSocketStreamCreateExtensionTest
} }
}; };
// TODO(https://crbug.com/842575) Re-enable testing with HTTP2_HANDSHAKE_STREAM.
INSTANTIATE_TEST_CASE_P(, INSTANTIATE_TEST_CASE_P(,
WebSocketStreamCreateExtensionTest, WebSocketStreamCreateExtensionTest,
Values(BASIC_HANDSHAKE_STREAM, HTTP2_HANDSHAKE_STREAM)); Values(BASIC_HANDSHAKE_STREAM));
// Common code to construct expectations for authentication tests that receive // Common code to construct expectations for authentication tests that receive
// the auth challenge on one connection and then create a second connection to // the auth challenge on one connection and then create a second connection to
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment