Skip to content

Commit

Permalink
http2: add h2 compat support for appendHeader
Browse files Browse the repository at this point in the history
PR-URL: #51412
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
  • Loading branch information
pimterry authored and targos committed Feb 15, 2024
1 parent d8aa2ba commit 4b1c683
Show file tree
Hide file tree
Showing 5 changed files with 114 additions and 8 deletions.
6 changes: 3 additions & 3 deletions doc/api/http.md
Original file line number Diff line number Diff line change
Expand Up @@ -2991,12 +2991,12 @@ added:
* `value` {string|string\[]} Header value
* Returns: {this}

Append a single header value for the header object.
Append a single header value to the header object.

If the value is an array, this is equivalent of calling this method multiple
If the value is an array, this is equivalent to calling this method multiple
times.

If there were no previous value for the header, this is equivalent of calling
If there were no previous values for the header, this is equivalent to calling
[`outgoingMessage.setHeader(name, value)`][].

Depending of the value of `options.uniqueHeaders` when the client request or the
Expand Down
30 changes: 30 additions & 0 deletions doc/api/http2.md
Original file line number Diff line number Diff line change
Expand Up @@ -3659,6 +3659,36 @@ message) to the response.
Attempting to set a header field name or value that contains invalid characters
will result in a [`TypeError`][] being thrown.

#### `response.appendHeader(name, value)`

<!-- YAML
added: REPLACEME
-->

* `name` {string}
* `value` {string|string\[]}

Append a single header value to the header object.

If the value is an array, this is equivalent to calling this method multiple
times.

If there were no previous values for the header, this is equivalent to calling
[`response.setHeader()`][].

Attempting to set a header field name or value that contains invalid characters
will result in a [`TypeError`][] being thrown.

```js
// Returns headers including "set-cookie: a" and "set-cookie: b"
const server = http2.createServer((req, res) => {
res.setHeader('set-cookie', 'a');
res.appendHeader('set-cookie', 'b');
res.writeHead(200);
res.end('ok');
});
```

#### `response.connection`

<!-- YAML
Expand Down
69 changes: 67 additions & 2 deletions lib/internal/http2/compat.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ const kRawHeaders = Symbol('rawHeaders');
const kTrailers = Symbol('trailers');
const kRawTrailers = Symbol('rawTrailers');
const kSetHeader = Symbol('setHeader');
const kAppendHeader = Symbol('appendHeader');
const kAborted = Symbol('aborted');

let statusMessageWarned = false;
Expand Down Expand Up @@ -652,6 +653,47 @@ class Http2ServerResponse extends Stream {
this[kHeaders][name] = value;
}

appendHeader(name, value) {
validateString(name, 'name');
if (this[kStream].headersSent)
throw new ERR_HTTP2_HEADERS_SENT();

this[kAppendHeader](name, value);
}

[kAppendHeader](name, value) {
name = StringPrototypeToLowerCase(StringPrototypeTrim(name));
assertValidHeader(name, value);

if (!isConnectionHeaderAllowed(name, value)) {
return;
}

if (name[0] === ':')
assertValidPseudoHeader(name);
else if (!checkIsHttpToken(name))
this.destroy(new ERR_INVALID_HTTP_TOKEN('Header name', name));

// Handle various possible cases the same as OutgoingMessage.appendHeader:
const headers = this[kHeaders];
if (headers === null || !headers[name]) {
return this.setHeader(name, value);
}

if (!ArrayIsArray(headers[name])) {
headers[name] = [headers[name]];
}

const existingValues = headers[name];
if (ArrayIsArray(value)) {
for (let i = 0, length = value.length; i < length; i++) {
existingValues.push(value[i]);
}
} else {
existingValues.push(value);
}
}

get statusMessage() {
statusMessageWarn();

Expand Down Expand Up @@ -684,18 +726,41 @@ class Http2ServerResponse extends Stream {

let i;
if (ArrayIsArray(headers)) {
if (this[kHeaders]) {
// Headers in obj should override previous headers but still
// allow explicit duplicates. To do so, we first remove any
// existing conflicts, then use appendHeader. This is the
// slow path, which only applies when you use setHeader and
// then pass headers in writeHead too.

// We need to handle both the tuple and flat array formats, just
// like the logic further below.
if (headers.length && ArrayIsArray(headers[0])) {
for (let n = 0; n < headers.length; n += 1) {
const key = headers[n + 0][0];
this.removeHeader(key);
}
} else {
for (let n = 0; n < headers.length; n += 2) {
const key = headers[n + 0];
this.removeHeader(key);
}
}
}

// Append all the headers provided in the array:
if (headers.length && ArrayIsArray(headers[0])) {
for (i = 0; i < headers.length; i++) {
const header = headers[i];
this[kSetHeader](header[0], header[1]);
this[kAppendHeader](header[0], header[1]);
}
} else {
if (headers.length % 2 !== 0) {
throw new ERR_INVALID_ARG_VALUE('headers', headers);
}

for (i = 0; i < headers.length; i += 2) {
this[kSetHeader](headers[i], headers[i + 1]);
this[kAppendHeader](headers[i], headers[i + 1]);
}
}
} else if (typeof headers === 'object') {
Expand Down
10 changes: 10 additions & 0 deletions test/parallel/test-http2-compat-serverresponse-headers.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,18 @@ server.listen(0, common.mustCall(function() {
response.setHeader(denormalised, expectedValue);
assert.strictEqual(response.getHeader(denormalised), expectedValue);
assert.strictEqual(response.hasHeader(denormalised), true);
assert.strictEqual(response.hasHeader(real), true);

response.appendHeader(real, expectedValue);
assert.deepStrictEqual(response.getHeader(real), [
expectedValue,
expectedValue,
]);
assert.strictEqual(response.hasHeader(real), true);

response.removeHeader(denormalised);
assert.strictEqual(response.hasHeader(denormalised), false);
assert.strictEqual(response.hasHeader(real), false);

['hasHeader', 'getHeader', 'removeHeader'].forEach((fnName) => {
assert.throws(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ const http2 = require('http2');
server.once('request', common.mustCall((request, response) => {
const returnVal = response.writeHead(200, [
['foo', 'bar'],
['foo', 'baz'],
['ABC', 123],
]);
assert.strictEqual(returnVal, response);
Expand All @@ -26,7 +27,7 @@ const http2 = require('http2');
const request = client.request();

request.on('response', common.mustCall((headers) => {
assert.strictEqual(headers.foo, 'bar');
assert.strictEqual(headers.foo, 'bar, baz');
assert.strictEqual(headers.abc, '123');
assert.strictEqual(headers[':status'], 200);
}, 1));
Expand All @@ -45,7 +46,7 @@ const http2 = require('http2');
const port = server.address().port;

server.once('request', common.mustCall((request, response) => {
const returnVal = response.writeHead(200, ['foo', 'bar', 'ABC', 123]);
const returnVal = response.writeHead(200, ['foo', 'bar', 'foo', 'baz', 'ABC', 123]);
assert.strictEqual(returnVal, response);
response.end(common.mustCall(() => { server.close(); }));
}));
Expand All @@ -54,7 +55,7 @@ const http2 = require('http2');
const request = client.request();

request.on('response', common.mustCall((headers) => {
assert.strictEqual(headers.foo, 'bar');
assert.strictEqual(headers.foo, 'bar, baz');
assert.strictEqual(headers.abc, '123');
assert.strictEqual(headers[':status'], 200);
}, 1));
Expand Down

0 comments on commit 4b1c683

Please sign in to comment.