Skip to content

Commit

Permalink
http2: make writeHead behave like HTTP/1.
Browse files Browse the repository at this point in the history
Backport-PR-URL: #14813
Backport-Reviewed-By: Anna Henningsen <[email protected]>
Backport-Reviewed-By: Timothy Gu <[email protected]>

PR-URL: #14239
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
  • Loading branch information
mcollina authored and addaleax committed Aug 14, 2017
1 parent b778838 commit 7824fa0
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 34 deletions.
2 changes: 1 addition & 1 deletion doc/api/http2.md
Original file line number Diff line number Diff line change
Expand Up @@ -2402,7 +2402,7 @@ buffer. Returns `false` if all or part of the data was queued in user memory.
added: REPLACEME
-->

Does nothing. Added for parity with [HTTP/1]().
Throws an error as the `'continue'` flow is not current implemented. Added for parity with [HTTP/1]().

### response.writeHead(statusCode[, statusMessage][, headers])
<!-- YAML
Expand Down
43 changes: 15 additions & 28 deletions lib/internal/http2/compat.js
Original file line number Diff line number Diff line change
Expand Up @@ -386,11 +386,6 @@ class Http2ServerResponse extends Stream {
headers[name] = String(value);
}

flushHeaders() {
if (this[kStream].headersSent === false)
this[kBeginSend]();
}

get statusMessage() {
if (statusMessageWarned === false) {
process.emitWarning(
Expand All @@ -403,6 +398,11 @@ class Http2ServerResponse extends Stream {
return '';
}

flushHeaders() {
if (this[kStream].headersSent === false)
this[kBeginSend]();
}

writeHead(statusCode, statusMessage, headers) {
if (typeof statusMessage === 'string' && statusMessageWarned === false) {
process.emitWarning(
Expand All @@ -414,6 +414,12 @@ class Http2ServerResponse extends Stream {
if (headers === undefined && typeof statusMessage === 'object') {
headers = statusMessage;
}

const stream = this[kStream];
if (stream.headersSent === true) {
throw new errors.Error('ERR_HTTP2_INFO_HEADERS_AFTER_RESPOND');
}

if (headers) {
const keys = Object.keys(headers);
let key = '';
Expand All @@ -422,8 +428,9 @@ class Http2ServerResponse extends Stream {
this.setHeader(key, headers[key]);
}
}

this.statusCode = statusCode;
// TODO mcollina this should probably call sendInfo
this[kBeginSend]();
}

write(chunk, encoding, cb) {
Expand Down Expand Up @@ -487,26 +494,6 @@ class Http2ServerResponse extends Stream {
stream.setTimeout(msecs, callback);
}

sendContinue(headers) {
this.sendInfo(100, headers);
}

sendInfo(code, headers) {
const stream = this[kStream];
if (stream.headersSent === true) {
throw new errors.Error('ERR_HTTP2_INFO_HEADERS_AFTER_RESPOND');
}
if (headers && typeof headers !== 'object')
throw new errors.TypeError('ERR_HTTP2_HEADERS_OBJECT');
if (stream === undefined) return;
code |= 0;
if (code < 100 || code >= 200)
throw new errors.RangeError('ERR_HTTP2_INVALID_INFO_STATUS', code);

headers[constants.HTTP2_HEADER_STATUS] = code;
stream.respond(headers);
}

createPushResponse(headers, callback) {
const stream = this[kStream];
if (stream === undefined) {
Expand Down Expand Up @@ -544,9 +531,9 @@ class Http2ServerResponse extends Stream {
this.emit('finish');
}

// added for parity with HTTP/1
writeContinue() {
// TODO mcollina this should probably be sendContinue
// TODO mcollina check what is the continue flow
throw new Error('not implemented yet');
}
}

Expand Down
1 change: 0 additions & 1 deletion test/parallel/test-http2-compat-serverresponse-end.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ const {
const server = createServer(mustCall((request, response) => {
strictEqual(response.finished, true);
response.writeHead(HTTP_STATUS_OK, { foo: 'bar' });
response.flushHeaders();
response.end(mustNotCall());
}));
server.listen(0, mustCall(() => {
Expand Down
11 changes: 9 additions & 2 deletions test/parallel/test-http2-compat-serverresponse-flushheaders.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,24 @@ const h2 = require('http2');

// Http2ServerResponse.flushHeaders

let serverResponse;

const server = h2.createServer();
server.listen(0, common.mustCall(function() {
const port = server.address().port;
server.once('request', common.mustCall(function(request, response) {
response.flushHeaders();
response.flushHeaders(); // Idempotent
response.writeHead(400, { 'foo-bar': 'abc123' }); // Ignored
common.expectsError(() => {
response.writeHead(400, { 'foo-bar': 'abc123' });
}, {
code: 'ERR_HTTP2_INFO_HEADERS_AFTER_RESPOND'
});

response.on('finish', common.mustCall(function() {
server.close();
}));
response.end();
serverResponse = response;
}));

const url = `http://localhost:${port}`;
Expand All @@ -33,6 +39,7 @@ server.listen(0, common.mustCall(function() {
request.on('response', common.mustCall(function(headers, flags) {
assert.strictEqual(headers['foo-bar'], undefined);
assert.strictEqual(headers[':status'], 200);
serverResponse.end();
}, 1));
request.on('end', common.mustCall(function() {
client.destroy();
Expand Down
6 changes: 4 additions & 2 deletions test/parallel/test-http2-compat-serverresponse-writehead.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,13 @@ server.listen(0, common.mustCall(function() {
const port = server.address().port;
server.once('request', common.mustCall(function(request, response) {
response.setHeader('foo-bar', 'def456');
response.writeHead(500);
response.writeHead(418, { 'foo-bar': 'abc123' }); // Override

common.expectsError(() => { response.writeHead(300); }, {
code: 'ERR_HTTP2_INFO_HEADERS_AFTER_RESPOND'
});

response.on('finish', common.mustCall(function() {
assert.doesNotThrow(() => { response.writeHead(300); });
server.close();
}));
response.end();
Expand Down

0 comments on commit 7824fa0

Please sign in to comment.