feat: Ignore invalid header parts

* patch: initial ideas around how to ignore invalid accept header parts. not fully tested, plenty of cleanup to do

* patch: finish removing commented code. finalize unit tests for headerUtil methods that now ignore bad header parts rather than throwing as before

* patch: remove @throws comments in HeaderUtil and update some comments to account for changes to returned values

* feat: Give option for header parsing to be strict

---------

Co-authored-by: Charlie Blevins <blevins.charlie@gmail.com>
This commit is contained in:
Joachim Van Herwegen 2023-07-20 15:27:20 +02:00 committed by GitHub
parent 6f6784a288
commit 9c2c5edaf5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 211 additions and 130 deletions

View File

@ -140,6 +140,9 @@ const mediaRange = new RegExp(`${tchar.source}+/${tchar.source}+`, 'u');
* Replaces all double quoted strings in the input string with `"0"`, `"1"`, etc.
* @param input - The Accept header string.
*
* @throws {@link BadRequestHttpError}
* Thrown if invalid characters are detected in a quoted string.
*
* @returns The transformed string and a map with keys `"0"`, etc. and values the original string that was there.
*/
export function transformQuotedStrings(input: string): { result: string; replacements: Record<string, string> } {
@ -163,6 +166,8 @@ export function transformQuotedStrings(input: string): { result: string; replace
* Splits the input string on commas, trims all parts and filters out empty ones.
*
* @param input - Input header string.
*
* @returns An array of trimmed strings.
*/
export function splitAndClean(input: string): string[] {
return input.split(',')
@ -175,44 +180,67 @@ export function splitAndClean(input: string): string[] {
*
* @param qvalue - Input qvalue string (so "q=....").
*
* @throws {@link BadRequestHttpError}
* Thrown on invalid syntax.
* @returns true if q value is valid, false otherwise.
*/
function testQValue(qvalue: string): void {
if (!/^(?:(?:0(?:\.\d{0,3})?)|(?:1(?:\.0{0,3})?))$/u.test(qvalue)) {
logger.warn(`Invalid q value: ${qvalue}`);
throw new BadRequestHttpError(
`Invalid q value: ${qvalue} does not match ( "0" [ "." 0*3DIGIT ] ) / ( "1" [ "." 0*3("0") ] ).`,
);
function isValidQValue(qvalue: string): boolean {
return /^(?:(?:0(?:\.\d{0,3})?)|(?:1(?:\.0{0,3})?))$/u.test(qvalue);
}
/**
* Converts a qvalue to a number.
* Returns 1 if the value is not a valid number or 1 if it is more than 1.
* Returns 0 if the value is negative.
* Otherwise, the parsed value is returned.
*
* @param qvalue - Value to convert.
*/
function parseQValue(qvalue: string): number {
const result = Number(qvalue);
if (Number.isNaN(result) || result >= 1) {
return 1;
}
if (result < 0) {
return 0;
}
return result;
}
/**
* Logs a warning to indicate there was an invalid value.
* Throws a {@link BadRequestHttpError} in case `strict` is `true`.
*
* @param message - Message to log and potentially put in the error.
* @param strict - `true` if an error needs to be thrown.
*/
function handleInvalidValue(message: string, strict: boolean): void | never {
logger.warn(message);
if (strict) {
throw new BadRequestHttpError(message);
}
}
/**
* Parses a list of split parameters and checks their validity.
* Parses a list of split parameters and checks their validity. Parameters with invalid
* syntax are ignored and not returned.
*
* @param parameters - A list of split parameters (token [ "=" ( token / quoted-string ) ])
* @param replacements - The double quoted strings that need to be replaced.
*
*
* @throws {@link BadRequestHttpError}
* Thrown on invalid parameter syntax.
* @param strict - Determines if invalid values throw errors (`true`) or log warnings (`false`). Defaults to `false`.
*
* @returns An array of name/value objects corresponding to the parameters.
*/
export function parseParameters(parameters: string[], replacements: Record<string, string>):
export function parseParameters(parameters: string[], replacements: Record<string, string>, strict = false):
{ name: string; value: string }[] {
return parameters.map((param): { name: string; value: string } => {
return parameters.reduce<{ name: string; value: string }[]>((acc, param): { name: string; value: string }[] => {
const [ name, rawValue ] = param.split('=').map((str): string => str.trim());
// Test replaced string for easier check
// parameter = token "=" ( token / quoted-string )
// second part is optional for certain parameters
if (!(token.test(name) && (!rawValue || /^"\d+"$/u.test(rawValue) || token.test(rawValue)))) {
logger.warn(`Invalid parameter value: ${name}=${replacements[rawValue] || rawValue}`);
throw new BadRequestHttpError(
`Invalid parameter value: ${name}=${replacements[rawValue] || rawValue} ` +
`does not match (token ( "=" ( token / quoted-string ))?). `,
);
handleInvalidValue(`Invalid parameter value: ${name}=${replacements[rawValue] || rawValue} ` +
`does not match (token ( "=" ( token / quoted-string ))?). `, strict);
return acc;
}
let value = rawValue;
@ -220,8 +248,9 @@ export function parseParameters(parameters: string[], replacements: Record<strin
value = replacements[rawValue];
}
return { name, value };
});
acc.push({ name, value });
return acc;
}, []);
}
/**
@ -229,24 +258,24 @@ export function parseParameters(parameters: string[], replacements: Record<strin
* For every parameter value that is a double quoted string,
* we check if it is a key in the replacements map.
* If yes the value from the map gets inserted instead.
* Invalid q values and parameter values are ignored and not returned.
*
* @param part - A string corresponding to a media range and its corresponding parameters.
* @param replacements - The double quoted strings that need to be replaced.
* @param strict - Determines if invalid values throw errors (`true`) or log warnings (`false`). Defaults to `false`.
*
* @throws {@link BadRequestHttpError}
* Thrown on invalid type, qvalue or parameter syntax.
*
* @returns {@link Accept} object corresponding to the header string.
* @returns {@link Accept | undefined} object corresponding to the header string, or
* undefined if an invalid type or sub-type is detected.
*/
function parseAcceptPart(part: string, replacements: Record<string, string>): Accept {
function parseAcceptPart(part: string, replacements: Record<string, string>, strict: boolean): Accept | undefined {
const [ range, ...parameters ] = part.split(';').map((param): string => param.trim());
// No reason to test differently for * since we don't check if the type exists
if (!mediaRange.test(range)) {
logger.warn(`Invalid Accept range: ${range}`);
throw new BadRequestHttpError(
`Invalid Accept range: ${range} does not match ( "*/*" / ( token "/" "*" ) / ( token "/" token ) )`,
handleInvalidValue(
`Invalid Accept range: ${range} does not match ( "*/*" / ( token "/" "*" ) / ( token "/" token ) )`, strict,
);
return;
}
let weight = 1;
@ -258,13 +287,16 @@ function parseAcceptPart(part: string, replacements: Record<string, string>): Ac
if (name === 'q') {
// Extension parameters appear after the q value
map = extensionParams;
testQValue(value);
weight = Number.parseFloat(value);
if (!isValidQValue(value)) {
handleInvalidValue(`Invalid q value for range ${range}: ${value
} does not match ( "0" [ "." 0*3DIGIT ] ) / ( "1" [ "." 0*3("0") ] ).`, strict);
}
weight = parseQValue(value);
} else {
if (!value && map !== extensionParams) {
logger.warn(`Invalid Accept parameter ${name}`);
throw new BadRequestHttpError(`Invalid Accept parameter ${name}: ` +
`Accept parameter values are not optional when preceding the q value`);
handleInvalidValue(`Invalid Accept parameter ${name}: ` +
`Accept parameter values are not optional when preceding the q value`, strict);
return;
}
map[name] = value || '';
}
@ -282,14 +314,13 @@ function parseAcceptPart(part: string, replacements: Record<string, string>): Ac
/**
* Parses an Accept-* header where each part is only a value and a weight, so roughly /.*(q=.*)?/ separated by commas.
* The returned weights default to 1 if no q value is found or the q value is invalid.
* @param input - Input header string.
*
* @throws {@link BadRequestHttpError}
* Thrown on invalid qvalue syntax.
* @param strict - Determines if invalid values throw errors (`true`) or log warnings (`false`). Defaults to `false`.
*
* @returns An array of ranges and weights.
*/
function parseNoParameters(input: string): AcceptHeader[] {
function parseNoParameters(input: string, strict = false): AcceptHeader[] {
const parts = splitAndClean(input);
return parts.map((part): AcceptHeader => {
@ -297,12 +328,15 @@ function parseNoParameters(input: string): AcceptHeader[] {
const result = { range, weight: 1 };
if (qvalue) {
if (!qvalue.startsWith('q=')) {
logger.warn(`Only q parameters are allowed in ${input}`);
throw new BadRequestHttpError(`Only q parameters are allowed in ${input}`);
handleInvalidValue(`Only q parameters are allowed in ${input}`, strict);
return result;
}
const val = qvalue.slice(2);
testQValue(val);
result.weight = Number.parseFloat(val);
if (!isValidQValue(val)) {
handleInvalidValue(`Invalid q value for range ${range}: ${val
} does not match ( "0" [ "." 0*3DIGIT ] ) / ( "1" [ "." 0*3("0") ] ).`, strict);
}
result.weight = parseQValue(val);
}
return result;
}).sort((left, right): number => right.weight - left.weight);
@ -314,17 +348,25 @@ function parseNoParameters(input: string): AcceptHeader[] {
* Parses an Accept header string.
*
* @param input - The Accept header string.
* @param strict - Determines if invalid values throw errors (`true`) or log warnings (`false`). Defaults to `false`.
*
* @throws {@link BadRequestHttpError}
* Thrown on invalid header syntax.
*
* @returns An array of {@link Accept} objects, sorted by weight.
* @returns An array of {@link Accept} objects, sorted by weight. Accept parts
* with invalid syntax are ignored and removed from the returned array.
*/
export function parseAccept(input: string): Accept[] {
export function parseAccept(input: string, strict = false): Accept[] {
// Quoted strings could prevent split from having correct results
const { result, replacements } = transformQuotedStrings(input);
return splitAndClean(result)
.map((part): Accept => parseAcceptPart(part, replacements))
.reduce<Accept[]>((acc, part): Accept[] => {
const partOrUndef = parseAcceptPart(part, replacements, strict);
if (partOrUndef !== undefined) {
acc.push(partOrUndef);
}
return acc;
}, [])
.sort((left, right): number => right.weight - left.weight);
}
@ -332,70 +374,65 @@ export function parseAccept(input: string): Accept[] {
* Parses an Accept-Charset header string.
*
* @param input - The Accept-Charset header string.
* @param strict - Determines if invalid values throw errors (`true`) or log warnings (`false`). Defaults to `false`.
*
* @throws {@link BadRequestHttpError}
* Thrown on invalid header syntax.
*
* @returns An array of {@link AcceptCharset} objects, sorted by weight.
* @returns An array of {@link AcceptCharset} objects, sorted by weight. Invalid ranges
* are ignored and not returned.
*/
export function parseAcceptCharset(input: string): AcceptCharset[] {
export function parseAcceptCharset(input: string, strict = false): AcceptCharset[] {
const results = parseNoParameters(input);
results.forEach((result): void => {
return results.filter((result): boolean => {
if (!token.test(result.range)) {
logger.warn(`Invalid Accept-Charset range: ${result.range}`);
throw new BadRequestHttpError(
`Invalid Accept-Charset range: ${result.range} does not match (content-coding / "identity" / "*")`,
handleInvalidValue(
`Invalid Accept-Charset range: ${result.range} does not match (content-coding / "identity" / "*")`, strict,
);
return false;
}
return true;
});
return results;
}
/**
* Parses an Accept-Encoding header string.
*
* @param input - The Accept-Encoding header string.
* @param strict - Determines if invalid values throw errors (`true`) or log warnings (`false`). Defaults to `false`.
*
* @throws {@link BadRequestHttpError}
* Thrown on invalid header syntax.
*
* @returns An array of {@link AcceptEncoding} objects, sorted by weight.
* @returns An array of {@link AcceptEncoding} objects, sorted by weight. Invalid ranges
* are ignored and not returned.
*/
export function parseAcceptEncoding(input: string): AcceptEncoding[] {
export function parseAcceptEncoding(input: string, strict = false): AcceptEncoding[] {
const results = parseNoParameters(input);
results.forEach((result): void => {
return results.filter((result): boolean => {
if (!token.test(result.range)) {
logger.warn(`Invalid Accept-Encoding range: ${result.range}`);
throw new BadRequestHttpError(`Invalid Accept-Encoding range: ${result.range} does not match (charset / "*")`);
handleInvalidValue(`Invalid Accept-Encoding range: ${result.range} does not match (charset / "*")`, strict);
return false;
}
return true;
});
return results;
}
/**
* Parses an Accept-Language header string.
*
* @param input - The Accept-Language header string.
* @param strict - Determines if invalid values throw errors (`true`) or log warnings (`false`). Defaults to `false`.
*
* @throws {@link BadRequestHttpError}
* Thrown on invalid header syntax.
*
* @returns An array of {@link AcceptLanguage} objects, sorted by weight.
* @returns An array of {@link AcceptLanguage} objects, sorted by weight. Invalid ranges
* are ignored and not returned.
*/
export function parseAcceptLanguage(input: string): AcceptLanguage[] {
export function parseAcceptLanguage(input: string, strict = false): AcceptLanguage[] {
const results = parseNoParameters(input);
results.forEach((result): void => {
return results.filter((result): boolean => {
// (1*8ALPHA *("-" 1*8alphanum)) / "*"
if (result.range !== '*' && !/^[a-zA-Z]{1,8}(?:-[a-zA-Z0-9]{1,8})*$/u.test(result.range)) {
logger.warn(
`Invalid Accept-Language range: ${result.range}`,
);
throw new BadRequestHttpError(
`Invalid Accept-Language range: ${result.range} does not match ((1*8ALPHA *("-" 1*8alphanum)) / "*")`,
handleInvalidValue(
`Invalid Accept-Language range: ${result.range} does not match ((1*8ALPHA *("-" 1*8alphanum)) / "*")`, strict,
);
return false;
}
return true;
});
return results;
}
// eslint-disable-next-line max-len
@ -405,24 +442,21 @@ const rfc1123Date = /^(?:Mon|Tue|Wed|Thu|Fri|Sat|Sun), \d{2} (?:Jan|Feb|Mar|Apr|
* Parses an Accept-DateTime header string.
*
* @param input - The Accept-DateTime header string.
* @param strict - Determines if invalid values throw errors (`true`) or log warnings (`false`). Defaults to `false`.
*
* @returns An array with a single {@link AcceptDatetime} object.
* @returns An array with a single {@link AcceptDatetime} object,
* or an empty array if a range in an invalid format is detected.
*/
export function parseAcceptDateTime(input: string): AcceptDatetime[] {
const results: AcceptDatetime[] = [];
export function parseAcceptDateTime(input: string, strict = false): AcceptDatetime[] {
const range = input.trim();
if (range) {
if (!range) {
return [];
}
if (!rfc1123Date.test(range)) {
logger.warn(
`Invalid Accept-DateTime range: ${range}`,
);
throw new BadRequestHttpError(
`Invalid Accept-DateTime range: ${range} does not match the RFC1123 format`,
);
handleInvalidValue(`Invalid Accept-DateTime range: ${range} does not match the RFC1123 format`, strict);
return [];
}
results.push({ range, weight: 1 });
}
return results;
return [{ range, weight: 1 }];
}
/**

View File

@ -53,32 +53,63 @@ describe('HeaderUtil', (): void => {
]);
});
it('rejects Accept Headers with invalid types.', async(): Promise<void> => {
expect((): any => parseAccept('*')).toThrow('Invalid Accept range:');
expect((): any => parseAccept('"bad"/text')).toThrow('Invalid Accept range:');
expect((): any => parseAccept('*/\\bad')).toThrow('Invalid Accept range:');
expect((): any => parseAccept('*/*')).not.toThrow('Invalid Accept range:');
it('ignores Accept Headers with invalid types.', async(): Promise<void> => {
expect(parseAccept('*')).toEqual([]);
expect(parseAccept('"bad"/text')).toEqual([]);
expect(parseAccept('*/\\bad')).toEqual([]);
expect(parseAccept('*/*')).toEqual([{
parameters: { extension: {}, mediaType: {}}, range: '*/*', weight: 1,
}]);
});
it('rejects Accept Headers with invalid q values.', async(): Promise<void> => {
expect((): any => parseAccept('a/b; q=text')).toThrow('Invalid q value:');
expect((): any => parseAccept('a/b; q=0.1234')).toThrow('Invalid q value:');
expect((): any => parseAccept('a/b; q=1.1')).toThrow('Invalid q value:');
expect((): any => parseAccept('a/b; q=1.000')).not.toThrow();
expect((): any => parseAccept('a/b; q=0.123')).not.toThrow();
it('ignores the weight of Accept Headers with q values it can not parse.', async(): Promise<void> => {
expect(parseAccept('a/b; q=text')).toEqual([{
range: 'a/b', weight: 1, parameters: { extension: {}, mediaType: {}},
}]);
// Invalid Q value but can be parsed
expect(parseAccept('a/b; q=0.1234')).toEqual([{
range: 'a/b', weight: 0.1234, parameters: { extension: {}, mediaType: {}},
}]);
expect(parseAccept('a/b; q=1.1')).toEqual([{
range: 'a/b', weight: 1, parameters: { extension: {}, mediaType: {}},
}]);
expect(parseAccept('a/b; q=1.000')).toEqual([{
range: 'a/b', weight: 1, parameters: { extension: {}, mediaType: {}},
}]);
expect(parseAccept('a/b; q=-5')).toEqual([{
range: 'a/b', weight: 0, parameters: { extension: {}, mediaType: {}},
}]);
expect(parseAccept('a/b; q=0.123')).toEqual([{
range: 'a/b', weight: 0.123, parameters: { extension: {}, mediaType: {}},
}]);
});
it('rejects Accept Headers with invalid parameters.', async(): Promise<void> => {
expect((): any => parseAccept('a/b; a')).toThrow('Invalid Accept parameter');
expect((): any => parseAccept('a/b; a=\\')).toThrow('Invalid parameter value');
expect((): any => parseAccept('a/b; q=1 ; a=\\')).toThrow('Invalid parameter value');
expect((): any => parseAccept('a/b; q=1 ; a')).not.toThrow('Invalid Accept parameter');
it('ignores Accept Headers with invalid parameters.', async(): Promise<void> => {
expect(parseAccept('a/b; a')).toEqual([{
range: 'a/b', weight: 1, parameters: { extension: {}, mediaType: {}},
}]);
expect(parseAccept('a/b; a=\\')).toEqual([{
range: 'a/b', weight: 1, parameters: { extension: {}, mediaType: {}},
}]);
expect(parseAccept('a/b; q=1 ; a=\\')).toEqual([{
range: 'a/b', weight: 1, parameters: { extension: {}, mediaType: {}},
}]);
expect(parseAccept('a/b; q=1 ; a')).toEqual([{
// eslint-disable-next-line id-length
range: 'a/b', weight: 1, parameters: { extension: { a: '' }, mediaType: {}},
}]);
});
it('rejects Accept Headers with quoted parameters.', async(): Promise<void> => {
expect((): any => parseAccept('a/b; a="\\""')).not.toThrow();
expect((): any => parseAccept('a/b; a="\\\u007F"')).toThrow('Invalid quoted string in header:');
});
it('rejects invalid values when strict mode is enabled.', async(): Promise<void> => {
expect((): any => parseAccept('"bad"/text', true)).toThrow(BadRequestHttpError);
expect((): any => parseAccept('a/b; q=text', true)).toThrow(BadRequestHttpError);
expect((): any => parseAccept('a/b; a', true)).toThrow(BadRequestHttpError);
});
});
describe('#parseCharset', (): void => {
@ -89,10 +120,14 @@ describe('HeaderUtil', (): void => {
]);
});
it('rejects invalid Accept-Charset Headers.', async(): Promise<void> => {
expect((): any => parseAcceptCharset('a/b')).toThrow('Invalid Accept-Charset range:');
expect((): any => parseAcceptCharset('a; q=text')).toThrow('Invalid q value:');
expect((): any => parseAcceptCharset('a; c=d')).toThrow('Only q parameters are allowed');
it('ignores invalid Accept-Charset Headers.', async(): Promise<void> => {
expect(parseAcceptCharset('a/b')).toEqual([]);
expect(parseAcceptCharset('a; q=text')).toEqual([{ range: 'a', weight: 1 }]);
expect(parseAcceptCharset('a; c=d')).toEqual([{ range: 'a', weight: 1 }]);
});
it('rejects invalid values when strict mode is enabled.', async(): Promise<void> => {
expect((): any => parseAcceptCharset('a/b', true)).toThrow(BadRequestHttpError);
});
});
@ -109,10 +144,14 @@ describe('HeaderUtil', (): void => {
]);
});
it('rejects invalid Accept-Encoding Headers.', async(): Promise<void> => {
expect((): any => parseAcceptEncoding('a/b')).toThrow('Invalid Accept-Encoding range:');
expect((): any => parseAcceptEncoding('a; q=text')).toThrow('Invalid q value:');
expect((): any => parseAcceptCharset('a; c=d')).toThrow('Only q parameters are allowed');
it('ignores invalid Accept-Encoding Headers.', async(): Promise<void> => {
expect(parseAcceptEncoding('a/b')).toEqual([]);
expect(parseAcceptEncoding('a; q=text')).toEqual([{ range: 'a', weight: 1 }]);
expect(parseAcceptCharset('a; c=d')).toEqual([{ range: 'a', weight: 1 }]);
});
it('rejects invalid values when strict mode is enabled.', async(): Promise<void> => {
expect((): any => parseAcceptEncoding('a/b', true)).toThrow(BadRequestHttpError);
});
});
@ -125,16 +164,20 @@ describe('HeaderUtil', (): void => {
]);
});
it('rejects invalid Accept-Language Headers.', async(): Promise<void> => {
expect((): any => parseAcceptLanguage('a/b')).toThrow('Invalid Accept-Language range:');
expect((): any => parseAcceptLanguage('05-a')).toThrow('Invalid Accept-Language range:');
expect((): any => parseAcceptLanguage('a--05')).toThrow('Invalid Accept-Language range:');
expect((): any => parseAcceptLanguage('a-"a"')).toThrow('Invalid Accept-Language range:');
expect((): any => parseAcceptLanguage('a-05')).not.toThrow('Invalid Accept-Language range:');
expect((): any => parseAcceptLanguage('a-b-c-d')).not.toThrow('Invalid Accept-Language range:');
it('ignores invalid Accept-Language Headers.', async(): Promise<void> => {
expect(parseAcceptLanguage('a/b')).toEqual([]);
expect(parseAcceptLanguage('05-a')).toEqual([]);
expect(parseAcceptLanguage('a--05')).toEqual([]);
expect(parseAcceptLanguage('a-"a"')).toEqual([]);
expect(parseAcceptLanguage('a-05')).toEqual([{ range: 'a-05', weight: 1 }]);
expect(parseAcceptLanguage('a-b-c-d')).toEqual([{ range: 'a-b-c-d', weight: 1 }]);
expect((): any => parseAcceptLanguage('a; q=text')).toThrow('Invalid q value:');
expect((): any => parseAcceptCharset('a; c=d')).toThrow('Only q parameters are allowed');
expect(parseAcceptLanguage('a; q=text')).toEqual([{ range: 'a', weight: 1 }]);
expect(parseAcceptCharset('a; c=d')).toEqual([{ range: 'a', weight: 1 }]);
});
it('rejects invalid values when strict mode is enabled.', async(): Promise<void> => {
expect((): any => parseAcceptLanguage('a/b', true)).toThrow(BadRequestHttpError);
});
});
@ -150,9 +193,13 @@ describe('HeaderUtil', (): void => {
expect(parseAcceptDateTime(' ')).toEqual([]);
});
it('rejects invalid Accept-DateTime Headers.', async(): Promise<void> => {
expect((): any => parseAcceptDateTime('a/b')).toThrow('Invalid Accept-DateTime range:');
expect((): any => parseAcceptDateTime('30 May 2007')).toThrow('Invalid Accept-DateTime range:');
it('ignores invalid Accept-DateTime Headers.', async(): Promise<void> => {
expect(parseAcceptDateTime('a/b')).toEqual([]);
expect(parseAcceptDateTime('30 May 2007')).toEqual([]);
});
it('rejects invalid values when strict mode is enabled.', async(): Promise<void> => {
expect((): any => parseAcceptLanguage('a/b', true)).toThrow(BadRequestHttpError);
});
});