mirror of
https://github.com/bilawalsidhu/gods-eye-view.git
synced 2026-11-09 21:02:19 +00:00
fix(server): read the whole q value, not a numeric prefix
Number.parseFloat accepts any valid numeric prefix, so `gzip;q=1oops` read as 1 and the out-of-range `gzip;q=2` was accepted — both contradicting the comment two lines above, which promises that a quality this cannot read confidently fails closed. Matching the qvalue grammar of RFC 9110 section 12.4.2 makes the code do what it already claimed to do. Thanks to kvnloo for catching it on review.
This commit is contained in:
1 parent
47b13ab18e
commit
e7dd71e9bc
2 files changed
+29
-9
No files matched your search
@@ -23,12 +23,20 @@ const gzipAsync = promisify(gzip);
|
||||
/** Below this, a gzip frame costs more than the bytes it saves. */
|
||||
const MIN_COMPRESSED_BYTES = 1024;
|
||||
|
||||
/**
|
||||
* The `qvalue` grammar of RFC 9110 section 12.4.2: zero to three decimals
|
||||
* below one, or one with only zeros after it. Matching the whole token rather
|
||||
* than parsing a numeric prefix is what makes `q=1oops` and the out-of-range
|
||||
* `q=2` unreadable instead of quietly rounding to something acceptable.
|
||||
*/
|
||||
const QVALUE = /^(?:0(?:\.\d{0,3})?|1(?:\.0{0,3})?)$/;
|
||||
|
||||
/**
|
||||
* Whether the client offered gzip in `Accept-Encoding`.
|
||||
*
|
||||
* Anything this cannot read confidently — a missing header, a malformed
|
||||
* quality value, the `x-gzip` alias — reads as "no", because the fallback is an
|
||||
* identity response every client can decode.
|
||||
* Anything this cannot read confidently — a missing header, a malformed or
|
||||
* out-of-range quality value, the `x-gzip` alias — reads as "no", because the
|
||||
* fallback is an identity response every client can decode.
|
||||
*
|
||||
* @param {import('node:http').IncomingMessage} request
|
||||
* @returns {boolean}
|
||||
@@ -39,12 +47,13 @@ export function acceptsGzip(request) {
|
||||
return header.split(',').some((part) => {
|
||||
const [token, ...parameters] = part.split(';');
|
||||
if (token.trim().toLowerCase() !== 'gzip') return false;
|
||||
// `q=0` is the one way a client can name an encoding and still refuse it
|
||||
// (RFC 9110 section 12.4.2).
|
||||
const quality = parameters
|
||||
.map((parameter) => parameter.trim().toLowerCase())
|
||||
.find((parameter) => parameter.startsWith('q='));
|
||||
return quality === undefined || Number.parseFloat(quality.slice(2)) > 0;
|
||||
if (quality === undefined) return true;
|
||||
// `q=0` is the one way a client can name an encoding and still refuse it.
|
||||
const value = quality.slice(2);
|
||||
return QVALUE.test(value) && Number(value) > 0;
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
@@ -61,12 +61,23 @@ test('accept-encoding is read the way clients actually write it', () => {
|
||||
assert.equal(acceptsGzip({}), false);
|
||||
assert.equal(acceptsGzip(undefined), false);
|
||||
|
||||
// A quality of zero names gzip and refuses it (RFC 9110 section 12.4.2),
|
||||
// and an unreadable quality falls back to the encoding every client accepts.
|
||||
// A quality of zero names gzip and refuses it (RFC 9110 section 12.4.2).
|
||||
assert.equal(acceptsGzip(request('gzip;q=0')), false);
|
||||
assert.equal(acceptsGzip(request('gzip;q=0.000')), false);
|
||||
assert.equal(acceptsGzip(request('gzip;q=bogus')), false);
|
||||
assert.equal(acceptsGzip(request('gzip;q=0.001')), true);
|
||||
assert.equal(acceptsGzip(request('gzip;q=1')), true);
|
||||
assert.equal(acceptsGzip(request('gzip;q=1.000')), true);
|
||||
|
||||
// A quality that is not a whole `qvalue` is unreadable, not a number with
|
||||
// trailing noise: the grammar allows three decimals and a ceiling of 1, so
|
||||
// these must not round into acceptance.
|
||||
assert.equal(acceptsGzip(request('gzip;q=bogus')), false);
|
||||
assert.equal(acceptsGzip(request('gzip;q=1oops')), false);
|
||||
assert.equal(acceptsGzip(request('gzip;q=0.5abc')), false);
|
||||
assert.equal(acceptsGzip(request('gzip;q=2')), false);
|
||||
assert.equal(acceptsGzip(request('gzip;q=1.5')), false);
|
||||
assert.equal(acceptsGzip(request('gzip;q=0.0001')), false);
|
||||
assert.equal(acceptsGzip(request('gzip;q=')), false);
|
||||
|
||||
// `gzip` is not matched as a substring of a neighbouring token.
|
||||
assert.equal(acceptsGzip(request('x-gzip')), false);
|
||||
|
||||
Reference in new issue
Block a user