Bug 1715401 - Fix highly compressed brotli responses with unknown content-type being rejected r=necko-reviewers,valentin

Highly compressed brotli files contain meta blocks that don't produce
output from the first 512 Bytes. That is because the meta block header
might be longer than 512 Bytes.[1] NBLTYPESL, NBLTYPESI, NBLTYPESD,
NTREESL, NTREESD all can blow up the header size. Each at least by
2*256 bits.

With this solution the Content-Type would be always detected as
`octet-stream`. That might not be the correct one. If a different
Content-Type is required, the server can still specify it with the
`Content-Type`-Header.[2]

This solution looks directly at the error code to determine invalid
encoding instead of relying on number of output bytes.

This also doesn't throw an error anymore for truncated brotli documents
that don't produce any output. But since we generally allow truncated
formats[3], it makes the implementation a bit more consistent.

[1]: https://www.rfc-editor.org/rfc/rfc7932#section-9.2
[2]: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Type
[3]: https://bugzilla.mozilla.org/show_bug.cgi?id=1242904#c4

Differential Revision: https://phabricator.services.mozilla.com/D175460
This commit is contained in:
Manuel Bucher 2023-06-29 14:23:47 +00:00
parent c477bd9809
commit 2b36fbf1bb
5 changed files with 128 additions and 7 deletions

View File

@ -11325,6 +11325,15 @@
value: true
mirror: always
# Whether to allow truncated brotli with empty output. This also fixes
# throwing an erroring when receiving highly compressed brotli files when
# no content type is specified (Bug 1715401). This pref can be removed after
# some time (Bug 1841061)
- name: network.compress.allow_truncated_empty_brotli
type: RelaxedAtomicBool
value: true
mirror: always
# See the full list of values in nsICookieService.idl.
- name: network.cookie.cookieBehavior
type: RelaxedAtomicInt32

View File

@ -13,6 +13,7 @@
#include "nsComponentManagerUtils.h"
#include "nsThreadUtils.h"
#include "mozilla/Preferences.h"
#include "mozilla/StaticPrefs_network.h"
#include "mozilla/Logging.h"
#include "nsIForcePendingChannel.h"
#include "nsIRequest.h"
@ -160,8 +161,11 @@ nsHTTPCompressConv::OnStopRequest(nsIRequest* request, nsresult aStatus) {
if (fpChannel && !isPending) {
fpChannel->ForcePending(true);
}
if (mBrotli && (mBrotli->mTotalOut == 0) &&
!mBrotli->mBrotliStateIsStreamEnd) {
bool allowTruncatedEmpty =
StaticPrefs::network_compress_allow_truncated_empty_brotli();
if (mBrotli && ((allowTruncatedEmpty && NS_FAILED(mBrotli->mStatus)) ||
(!allowTruncatedEmpty && mBrotli->mTotalOut == 0 &&
!mBrotli->mBrotliStateIsStreamEnd))) {
status = NS_ERROR_INVALID_CONTENT_ENCODING;
}
LOG(("nsHttpCompresssConv %p onstop brotlihandler rv %" PRIx32 "\n", this,

View File

@ -0,0 +1,72 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
// This test file makes sure that we can decode brotli files when the
// Content-Type header is missing (Bug 1715401)
"use strict";
function emptyBrotli(metadata, response) {
response.setHeader("Content-Encoding", "br", false);
response.write("\x01\x03\x06\x03");
}
function largeEmptyBrotli(metadata, response) {
response.setHeader("Content-Encoding", "br", false);
response.write("\x01\x03" + "\x06".repeat(600) + "\x03");
}
const { HttpServer } = ChromeUtils.import("resource://testing-common/httpd.js");
XPCOMUtils.defineLazyGetter(this, "URL_EMPTY_BROTLI", function () {
return (
"http://localhost:" + httpServer.identity.primaryPort + "/empty-brotli"
);
});
XPCOMUtils.defineLazyGetter(this, "URL_LARGE_EMPTY_BROTLI", function () {
return (
"http://localhost:" +
httpServer.identity.primaryPort +
"/large-empty-brotli"
);
});
var httpServer = null;
add_task(async function check_brotli() {
httpServer = new HttpServer();
httpServer.registerPathHandler("/empty-brotli", emptyBrotli);
httpServer.registerPathHandler("/large-empty-brotli", largeEmptyBrotli);
httpServer.start(-1);
async function test(url) {
let chan = NetUtil.newChannel({ uri: url, loadUsingSystemPrincipal: true });
let [, response] = await new Promise(resolve => {
chan.asyncOpen(
new ChannelListener(
(req, buff) => {
resolve([req, buff]);
},
null,
CL_IGNORE_CL
)
);
});
return response;
}
equal(
await test(URL_EMPTY_BROTLI),
"",
"Should decode brotli even when brotli output is empty"
);
equal(
await test(URL_LARGE_EMPTY_BROTLI),
"",
"Should decode brotli even when the nsUnknownDecoder can't get any decoded output"
);
Services.prefs.clearUserPref("network.http.accept-encoding");
Services.prefs.clearUserPref("network.http.encoding.trustworthy_is_https");
await httpServer.stop();
});

View File

@ -40,13 +40,13 @@ var tests = [
datalen: 5, // the data length of the uncompressed document
},
// this is not a brotli document
// this is a truncated brotli document, producing no output bytes
{
url: "/test/cebrotli2",
flags: CL_EXPECT_GZIP | CL_EXPECT_FAILURE,
flags: CL_EXPECT_GZIP,
ce: "br",
body: [0x0b, 0x0a, 0x09],
datalen: 3,
datalen: 0,
},
// this is brotli but should come through as identity due to prefs
@ -58,6 +58,37 @@ var tests = [
datalen: 9,
},
// this is not a brotli document
{
url: "/test/cebrotli4",
flags: CL_EXPECT_GZIP | CL_EXPECT_FAILURE,
ce: "br",
body: [
0x01, 0xe6, 0x00, 0x76, 0x42, 0x10, 0x01, 0x1c, 0x24, 0x24, 0x3c, 0xd7,
0xd7, 0xd7, 0x01, 0x1c,
],
datalen: 16,
},
// this is a brotli document
{
url: "/test/cebrotli5",
flags: CL_EXPECT_GZIP,
ce: "br",
body: [0x0b, 0x00, 0x80, 0x4e, 0x03],
datalen: 1,
},
// this a truncated brotli document (missing the end marker)
// producing one output byte
{
url: "/test/cebrotli6",
flags: CL_EXPECT_GZIP,
ce: "br",
body: [0x0b, 0x00, 0x80, 0x4e],
datalen: 1,
},
];
function setupChannel(url) {
@ -71,6 +102,8 @@ function startIter() {
if (tests[index].url === "/test/cebrotli3") {
// this test wants to make sure we don't do brotli when not in a-e
prefs.setCharPref("network.http.accept-encoding", "gzip, deflate");
} else {
prefs.setCharPref("network.http.accept-encoding", "gzip, deflate, br");
}
var channel = setupChannel(tests[index].url);
channel.asyncOpen(
@ -80,7 +113,7 @@ function startIter() {
function completeIter(request, data, ctx) {
if (!(tests[index].flags & CL_EXPECT_FAILURE)) {
Assert.equal(data.length, tests[index].datalen);
Assert.equal(data.length, tests[index].datalen, "test " + index);
}
if (++index < tests.length) {
startIter();
@ -95,7 +128,6 @@ var cePref;
function run_test() {
prefs = Services.prefs;
cePref = prefs.getCharPref("network.http.accept-encoding");
prefs.setCharPref("network.http.accept-encoding", "gzip, deflate, br");
prefs.setBoolPref("network.http.encoding.trustworthy_is_https", false);
httpserver.registerPathHandler("/test/cegzip1", handler);
@ -103,6 +135,9 @@ function run_test() {
httpserver.registerPathHandler("/test/cebrotli1", handler);
httpserver.registerPathHandler("/test/cebrotli2", handler);
httpserver.registerPathHandler("/test/cebrotli3", handler);
httpserver.registerPathHandler("/test/cebrotli4", handler);
httpserver.registerPathHandler("/test/cebrotli5", handler);
httpserver.registerPathHandler("/test/cebrotli6", handler);
httpserver.start(-1);
startIter();

View File

@ -556,6 +556,7 @@ skip-if =
os == 'win' && msix # https://bugzilla.mozilla.org/show_bug.cgi?id=1807931
run-sequentially = http3server
[test_brotli_http.js]
[test_brotli_unknown_content_type.js]
[test_altsvc_http3.js]
skip-if =
true # Bug 1675008