Skip to content

Commit

Permalink
fix(node:http) Export validateHeaderName and validateHeaderValue
Browse files Browse the repository at this point in the history
…functions (#22616)

Modify `_http_outgoing.ts` to support the extended signature of
`validateHeaderName()` used since node v19.5.0/v18.14.0 by adding the
`label` parameter. (see:
https://nodejs.org/api/http.html#httpvalidateheadernamename-label)

Making both validation functions accessible as public exports of
`node:http`

Fixes: #22614
  • Loading branch information
mash-graz authored and nathanwhit committed Mar 14, 2024
1 parent df69c2b commit 6057ddd
Show file tree
Hide file tree
Showing 5 changed files with 94 additions and 16 deletions.
32 changes: 18 additions & 14 deletions ext/node/polyfills/_http_outgoing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -820,21 +820,25 @@ Object.defineProperty(OutgoingMessage.prototype, "_headerNames", {
),
});

export const validateHeaderName = hideStackFrames((name) => {
if (typeof name !== "string" || !name || !checkIsHttpToken(name)) {
throw new ERR_INVALID_HTTP_TOKEN("Header name", name);
}
});
export const validateHeaderName = hideStackFrames(
(name: string, label?: string): void => {
if (typeof name !== "string" || !name || !checkIsHttpToken(name)) {
throw new ERR_INVALID_HTTP_TOKEN(label || "Header name", name);
}
},
);

export const validateHeaderValue = hideStackFrames((name, value) => {
if (value === undefined) {
throw new ERR_HTTP_INVALID_HEADER_VALUE(value, name);
}
if (checkInvalidHeaderChar(value)) {
debug('Header "%s" contains invalid characters', name);
throw new ERR_INVALID_CHAR("header content", name);
}
});
export const validateHeaderValue = hideStackFrames(
(name: string, value: string): void => {
if (value === undefined) {
throw new ERR_HTTP_INVALID_HEADER_VALUE(value, name);
}
if (checkInvalidHeaderChar(value)) {
debug('Header "%s" contains invalid characters', name);
throw new ERR_INVALID_CHAR("header content", name);
}
},
);

export function parseUniqueHeadersOption(headers) {
if (!Array.isArray(headers)) {
Expand Down
5 changes: 5 additions & 0 deletions ext/node/polyfills/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import {
OutgoingMessage,
parseUniqueHeadersOption,
validateHeaderName,
validateHeaderValue,
} from "ext:deno_node/_http_outgoing.ts";
import { ok as assert } from "node:assert";
import { kOutHeaders } from "ext:deno_node/internal/http.ts";
Expand Down Expand Up @@ -1824,6 +1825,8 @@ export {
METHODS,
OutgoingMessage,
STATUS_CODES,
validateHeaderName,
validateHeaderValue,
};
export default {
Agent,
Expand All @@ -1840,4 +1843,6 @@ export default {
ServerResponse,
request,
get,
validateHeaderName,
validateHeaderValue,
};
1 change: 1 addition & 0 deletions tests/node_compat/config.jsonc
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,7 @@
// TODO(lev): ClientRequest.socket is not polyfilled so this test keeps
// failing
//"test-http-client-set-timeout.js",
"test-http-header-validators.js",
"test-http-localaddress.js",
// TODO(bartlomieju): temporarily disabled while we iterate on the HTTP client
// "test-http-outgoing-buffer.js",
Expand Down
69 changes: 69 additions & 0 deletions tests/node_compat/test/parallel/test-http-header-validators.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
// deno-fmt-ignore-file
// deno-lint-ignore-file

// Copyright Joyent and Node contributors. All rights reserved. MIT license.
// Taken from Node 18.12.1
// This file is automatically generated by `tools/node_compat/setup.ts`. Do not modify this file manually.

'use strict';
require('../common');
const assert = require('assert');
const { validateHeaderName, validateHeaderValue } = require('http');

// Expected static methods
isFunc(validateHeaderName, 'validateHeaderName');
isFunc(validateHeaderValue, 'validateHeaderValue');

// Expected to be useful as static methods
console.log('validateHeaderName');
// - when used with valid header names - should not throw
[
'user-agent',
'USER-AGENT',
'User-Agent',
'x-forwarded-for',
].forEach((name) => {
console.log('does not throw for "%s"', name);
validateHeaderName(name);
});

// - when used with invalid header names:
[
'איקס-פורוורד-פור',
'x-forwarded-fםr',
].forEach((name) => {
console.log('throws for: "%s"', name.slice(0, 50));
assert.throws(
() => validateHeaderName(name),
{ code: 'ERR_INVALID_HTTP_TOKEN' }
);
});

console.log('validateHeaderValue');
// - when used with valid header values - should not throw
[
['x-valid', 1],
['x-valid', '1'],
['x-valid', 'string'],
].forEach(([name, value]) => {
console.log('does not throw for "%s"', name);
validateHeaderValue(name, value);
});

// - when used with invalid header values:
[
// [header, value, expectedCode]
['x-undefined', undefined, 'ERR_HTTP_INVALID_HEADER_VALUE'],
['x-bad-char', 'לא תקין', 'ERR_INVALID_CHAR'],
].forEach(([name, value, code]) => {
console.log('throws %s for: "%s: %s"', code, name, value);
assert.throws(
() => validateHeaderValue(name, value),
{ code }
);
});

// Misc.
function isFunc(v, ttl) {
assert.ok(v.constructor === Function, `${ttl} is expected to be a function`);
}
3 changes: 1 addition & 2 deletions tools/node_compat/TODO.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

NOTE: This file should not be manually edited. Please edit `tests/node_compat/config.json` and run `deno task setup` in `tools/node_compat` dir instead.

Total: 2999
Total: 2998

- [abort/test-abort-backtrace.js](https://github.com/nodejs/node/tree/v18.12.1/test/abort/test-abort-backtrace.js)
- [abort/test-abort-fatal-error.js](https://github.com/nodejs/node/tree/v18.12.1/test/abort/test-abort-fatal-error.js)
Expand Down Expand Up @@ -1052,7 +1052,6 @@ Total: 2999
- [parallel/test-http-header-overflow.js](https://github.com/nodejs/node/tree/v18.12.1/test/parallel/test-http-header-overflow.js)
- [parallel/test-http-header-owstext.js](https://github.com/nodejs/node/tree/v18.12.1/test/parallel/test-http-header-owstext.js)
- [parallel/test-http-header-read.js](https://github.com/nodejs/node/tree/v18.12.1/test/parallel/test-http-header-read.js)
- [parallel/test-http-header-validators.js](https://github.com/nodejs/node/tree/v18.12.1/test/parallel/test-http-header-validators.js)
- [parallel/test-http-hex-write.js](https://github.com/nodejs/node/tree/v18.12.1/test/parallel/test-http-hex-write.js)
- [parallel/test-http-highwatermark.js](https://github.com/nodejs/node/tree/v18.12.1/test/parallel/test-http-highwatermark.js)
- [parallel/test-http-host-header-ipv6-fail.js](https://github.com/nodejs/node/tree/v18.12.1/test/parallel/test-http-host-header-ipv6-fail.js)
Expand Down

0 comments on commit 6057ddd

Please sign in to comment.