Skip to content

Commit

Permalink
Missing Http2ServerResponse.setHeaders() fixed
Browse files Browse the repository at this point in the history
Make it possible to invoke response.setHeaders() on
response object from `node:http2`

Fixes: nodejs#51573
  • Loading branch information
Imperat committed Jan 27, 2024
1 parent af3e2b2 commit 9d915fc
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 0 deletions.
10 changes: 10 additions & 0 deletions doc/api/http2.md
Original file line number Diff line number Diff line change
Expand Up @@ -3935,6 +3935,16 @@ const server = http2.createServer((req, res) => {
});
```

#### `response.setHeaders()`

* `headers` {Headers|Map}

Sets multiple header values for implicit headers.
`headers` must be an instance of [`Headers`][] or `Map`,
if a header already exists in the to-be-sent headers,
its value will be replaced.


#### `response.setTimeout(msecs[, callback])`

<!-- YAML
Expand Down
18 changes: 18 additions & 0 deletions lib/internal/http2/compat.js
Original file line number Diff line number Diff line change
Expand Up @@ -637,6 +637,24 @@ class Http2ServerResponse extends Stream {
this[kSetHeader](name, value);
}

setHeaders(headers) {
if (this[kStream].headersSent)
throw new ERR_HTTP2_HEADERS_SENT();

if (
!headers ||
ArrayIsArray(headers) ||
typeof headers.keys !== 'function' ||
typeof headers.get !== 'function'
) {
throw new ERR_INVALID_ARG_TYPE('headers', ['Headers', 'Map'], headers);
}

for (const key of headers.keys()) {
this.setHeader(key, headers.get(key));

This comment has been minimized.

Copy link
@danfuzz

danfuzz Jan 29, 2024

It looks like this won't properly handle Set-Cookie headers, which need to be passed to setHeader() explicitly as an array in order to work.

This same bug is present in the HTTP implementation. (I am about to file that as a bug.)

This comment has been minimized.

Copy link
@danfuzz
}
}

[kSetHeader](name, value) {
name = StringPrototypeToLowerCase(StringPrototypeTrim(name));
assertValidHeader(name, value);
Expand Down
46 changes: 46 additions & 0 deletions test/parallel/test-http2-server-set-headers.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
'use strict';

const common = require('../common');
if (!common.hasCrypto)
common.skip('missing crypto');
const assert = require('assert');
const http2 = require('http2');
const body =
'<html><head></head><body><h1>this is some data</h2></body></html>';

const server = http2.createServer((req, res) => {
res.setHeaders(new Map([
['foobar', 'baz'],
['X-POWERED-BY', 'node-test'],
['connection', 'connection-test'],
]));
res.end(body);
});

server.listen(0, common.mustCall(() => {
const client = http2.connect(`http://localhost:${server.address().port}`);
const headers = { ':path': '/' };
const req = client.request(headers);
req.setEncoding('utf8');
req.on('response', common.mustCall(function(headers) {
assert.strictEqual(headers.foobar, 'baz');
assert.strictEqual(headers['x-powered-by'], 'node-test');
}));

let data = '';
req.on('data', (d) => data += d);
req.on('end', () => {
assert.strictEqual(body, data);
server.close();
client.close();
});
req.end();
}));

const compatMsg = 'The provided connection header is not valid, ' +
'the value will be dropped from the header and ' +
'will never be in use.';

common.expectWarning('UnsupportedWarning', compatMsg);

server.on('error', common.mustNotCall());

0 comments on commit 9d915fc

Please sign in to comment.