Bug 1352146 - Don't allow status phrases in http/2. r?mcmanus
MozReview-Commit-ID: Cf30tUivhnB
--- a/netwerk/protocol/http/Http2Stream.cpp
+++ b/netwerk/protocol/http/Http2Stream.cpp
@@ -1017,16 +1017,29 @@ Http2Stream::ConvertResponseHeaders(Http
decompressor->GetStatus(statusString);
if (statusString.IsEmpty()) {
LOG3(("Http2Stream::ConvertResponseHeaders %p Error - no status\n", this));
return NS_ERROR_ILLEGAL_VALUE;
}
nsresult errcode;
httpResponseCode = statusString.ToInteger(&errcode);
+
+ // Ensure the :status is just an HTTP status code
+ // https://tools.ietf.org/html/rfc7540#section-8.1.2.4
+ // https://bugzilla.mozilla.org/show_bug.cgi?id=1352146
+ nsAutoCString parsedStatusString;
+ parsedStatusString.AppendInt(httpResponseCode);
+ if (!parsedStatusString.Equals(statusString)) {
+ LOG3(("Http2Stream::ConvertResposeHeaders %p status %s is not just a code",
+ this, statusString.BeginReading()));
+ // Results in stream reset with PROTOCOL_ERROR
+ return NS_ERROR_ILLEGAL_VALUE;
+ }
+
LOG3(("Http2Stream::ConvertResponseHeaders %p response code %d\n", this, httpResponseCode));
if (mIsTunnel) {
LOG3(("Http2Stream %p Tunnel Response code %d", this, httpResponseCode));
if ((httpResponseCode / 100) != 2) {
MapStreamToPlainText();
}
}
--- a/netwerk/test/unit/test_http2.js
+++ b/netwerk/test/unit/test_http2.js
@@ -944,16 +944,23 @@ function test_http2_push_userContext2()
function test_http2_push_userContext3() {
var chan = makeChan("https://localhost:" + serverPort + "/push.js");
chan.loadGroup = loadGroup;
chan.loadInfo.originAttributes = { userContextId: 1 };
var listener = new Http2PushListener(true);
chan.asyncOpen2(listener);
}
+function test_http2_status_phrase() {
+ var chan = makeChan("https://localhost:" + serverPort + "/statusphrase");
+ var listener = new Http2CheckListener();
+ listener.shouldSucceed = false;
+ chan.asyncOpen2(listener);
+}
+
function test_complete() {
resetPrefs();
do_test_pending();
httpserv.stop(do_test_finished);
do_test_pending();
httpserv2.stop(do_test_finished);
do_test_finished();
@@ -989,16 +996,17 @@ var tests = [ test_http2_post_big
, test_http2_patch
, test_http2_pushapi_1
, test_http2_continuations
, test_http2_blocking_download
, test_http2_illegalhpacksoft
, test_http2_illegalhpackhard
, test_http2_folded_header
, test_http2_empty_data
+ , test_http2_status_phrase
// Add new tests above here - best to add new tests before h1
// streams get too involved
// These next two must always come in this order
, test_http2_h11required_stream
, test_http2_h11required_session
, test_http2_retry_rst
, test_http2_wrongsuite
, test_http2_push_firstparty1
--- a/testing/xpcshell/moz-http2/moz-http2.js
+++ b/testing/xpcshell/moz-http2/moz-http2.js
@@ -793,16 +793,24 @@ function handleRequest(req, res) {
'content-length' : 1
});
pushe.end('1');
}
else if (u.pathname.substring(0,8) === "/origin-") { // test_origin.js coalescing
res.setHeader("x-client-port", req.remotePort);
}
+ else if (u.pathname === "/statusphrase") {
+ // Fortunately, the node-http2 API is dumb enough to allow this right on
+ // through, so we can easily test rejecting this on gecko's end.
+ res.writeHead("200 OK");
+ res.end(content);
+ return;
+ }
+
res.setHeader('Content-Type', 'text/html');
if (req.httpVersionMajor != 2) {
res.setHeader('Connection', 'close');
}
res.writeHead(200);
res.end(content);
}