Skip to content
Permalink
Browse files

http2: make HTTP2ServerResponse more streams compliant

HTTP2ServerResponse.write would behave differently than
both http1 and streams. This PR makes it more compliant
with stream.Writable behaviour.

PR-URL: #30964
Refs: #29529
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
  • Loading branch information
ronag authored and BridgeAR committed Dec 14, 2019
1 parent b193142 commit b9160351ecb2273f45da1f01a14e538f8ecb98bb
Showing with 74 additions and 39 deletions.
  1. +21 −8 lib/internal/http2/compat.js
  2. +53 −31 test/parallel/test-http2-compat-serverresponse-write.js
@@ -40,7 +40,8 @@ const {
ERR_HTTP2_STATUS_INVALID,
ERR_INVALID_ARG_VALUE,
ERR_INVALID_CALLBACK,
ERR_INVALID_HTTP_TOKEN
ERR_INVALID_HTTP_TOKEN,
ERR_STREAM_WRITE_AFTER_END
},
hideStackFrames
} = require('internal/errors');
@@ -439,6 +440,7 @@ class Http2ServerResponse extends Stream {
this[kState] = {
closed: false,
ending: false,
destroyed: false,
headRequest: false,
sendDate: true,
statusCode: HTTP_STATUS_OK,
@@ -649,23 +651,32 @@ class Http2ServerResponse extends Stream {
}

write(chunk, encoding, cb) {
const state = this[kState];

if (typeof encoding === 'function') {
cb = encoding;
encoding = 'utf8';
}

if (this[kState].closed) {
const err = new ERR_HTTP2_INVALID_STREAM();
let err;
if (state.ending) {
err = new ERR_STREAM_WRITE_AFTER_END();
} else if (state.closed) {
err = new ERR_HTTP2_INVALID_STREAM();
} else if (state.destroyed) {
return false;
}

if (err) {
if (typeof cb === 'function')
process.nextTick(cb, err);
else
throw err;
return;
this.destroy(err);
return false;
}

const stream = this[kStream];
if (!stream.headersSent)
this.writeHead(this[kState].statusCode);
this.writeHead(state.statusCode);
return stream.write(chunk, encoding, cb);
}

@@ -712,8 +723,10 @@ class Http2ServerResponse extends Stream {
}

destroy(err) {
if (this[kState].closed)
if (this[kState].destroyed)
return;

this[kState].destroyed = true;
this[kStream].destroy(err);
}

@@ -10,42 +10,64 @@ if (!hasCrypto)
skip('missing crypto');
const { createServer, connect } = require('http2');
const assert = require('assert');

const server = createServer();
server.listen(0, mustCall(() => {
const port = server.address().port;
const url = `http://localhost:${port}`;
const client = connect(url, mustCall(() => {
const request = client.request();
request.resume();
request.on('end', mustCall());
request.on('close', mustCall(() => {
client.close();
{
const server = createServer();
server.listen(0, mustCall(() => {
const port = server.address().port;
const url = `http://localhost:${port}`;
const client = connect(url, mustCall(() => {
const request = client.request();
request.resume();
request.on('end', mustCall());
request.on('close', mustCall(() => {
client.close();
}));
}));
}));

server.once('request', mustCall((request, response) => {
// response.write() returns true
assert(response.write('muahaha', 'utf8', mustCall()));
server.once('request', mustCall((request, response) => {
// response.write() returns true
assert(response.write('muahaha', 'utf8', mustCall()));

response.stream.close(0, mustCall(() => {
response.on('error', mustNotCall());
response.stream.close(0, mustCall(() => {
response.on('error', mustNotCall());

// response.write() without cb returns error
assert.throws(
() => { response.write('muahaha'); },
{
name: 'Error',
code: 'ERR_HTTP2_INVALID_STREAM',
message: 'The stream has been destroyed'
}
);
// response.write() without cb returns error
response.write('muahaha', mustCall((err) => {
assert.strictEqual(err.code, 'ERR_HTTP2_INVALID_STREAM');

// response.write() with cb returns falsy value
assert(!response.write('muahaha', mustCall()));
// response.write() with cb returns falsy value
assert(!response.write('muahaha', mustCall()));

client.destroy();
server.close();
}));
}));
}));
}));
}

{
// Http2ServerResponse.write ERR_STREAM_WRITE_AFTER_END
const server = createServer();
server.listen(0, mustCall(() => {
const port = server.address().port;
const url = `http://localhost:${port}`;
const client = connect(url, mustCall(() => {
const request = client.request();
request.resume();
request.on('end', mustCall());
request.on('close', mustCall(() => {
client.close();
}));
}));

client.destroy();
server.close();
server.once('request', mustCall((request, response) => {
response.end();
response.write('asd', mustCall((err) => {
assert.strictEqual(err.code, 'ERR_STREAM_WRITE_AFTER_END');
client.destroy();
server.close();
}));
}));
}));
}));
}

0 comments on commit b916035

Please sign in to comment.
You can’t perform that action at this time.