Skip to content

Commit

Permalink
http2: avoid bind and properly clean up in compat
Browse files Browse the repository at this point in the history
Backport-PR-URL: #22850
PR-URL: #20374
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
  • Loading branch information
ronag authored and BethGriggs committed Oct 16, 2018
1 parent 4f00354 commit d1b7825
Showing 1 changed file with 46 additions and 32 deletions.
78 changes: 46 additions & 32 deletions lib/internal/http2/compat.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ const constants = binding.constants;
const errors = require('internal/errors');
const { kSocket } = require('internal/http2/util');

const kFinish = Symbol('finish');
const kBeginSend = Symbol('begin-send');
const kState = Symbol('state');
const kStream = Symbol('stream');
Expand Down Expand Up @@ -211,6 +210,27 @@ const proxySocketHandler = {
}
};

function onStreamCloseRequest() {
const req = this[kRequest];

if (req === undefined)
return;

const state = req[kState];
state.closed = true;

req.push(null);
// if the user didn't interact with incoming data and didn't pipe it,
// dump it for compatibility with http1
if (!state.didRead && !req._readableState.resumeScheduled)
req.resume();

this[kProxySocket] = null;
this[kRequest] = undefined;

req.emit('close');
}

class Http2ServerRequest extends Readable {
constructor(stream, headers, options, rawHeaders) {
super(options);
Expand All @@ -234,7 +254,7 @@ class Http2ServerRequest extends Readable {
stream.on('end', onStreamEnd);
stream.on('error', onStreamError);
stream.on('aborted', onStreamAbortedRequest);
stream.on('close', this[kFinish].bind(this));
stream.on('close', onStreamCloseRequest);
this.on('pause', onRequestPause);
this.on('resume', onRequestResume);
}
Expand Down Expand Up @@ -335,24 +355,30 @@ class Http2ServerRequest extends Readable {
return;
this[kStream].setTimeout(msecs, callback);
}

[kFinish]() {
const state = this[kState];
if (state.closed)
return;
state.closed = true;
this.push(null);
this[kStream][kRequest] = undefined;
// if the user didn't interact with incoming data and didn't pipe it,
// dump it for compatibility with http1
if (!state.didRead && !this._readableState.resumeScheduled)
this.resume();
this.emit('close');
}
}

function onStreamTrailersReady() {
this[kStream].sendTrailers(this[kTrailers]);
this.sendTrailers(this[kResponse][kTrailers]);
}

function onStreamCloseResponse() {
const res = this[kResponse];

if (res === undefined)
return;

const state = res[kState];

if (this.headRequest !== state.headRequest)
return;

state.closed = true;

this[kProxySocket] = null;
this[kResponse] = undefined;

res.emit('finish');
res.emit('close');
}

class Http2ServerResponse extends Stream {
Expand All @@ -373,8 +399,8 @@ class Http2ServerResponse extends Stream {
this.writable = true;
stream.on('drain', onStreamDrain);
stream.on('aborted', onStreamAbortedResponse);
stream.on('close', this[kFinish].bind(this));
stream.on('wantTrailers', onStreamTrailersReady.bind(this));
stream.on('close', onStreamCloseResponse);
stream.on('wantTrailers', onStreamTrailersReady);
}

// User land modules such as finalhandler just check truthiness of this
Expand Down Expand Up @@ -605,7 +631,7 @@ class Http2ServerResponse extends Stream {
this.writeHead(this[kState].statusCode);

if (isFinished)
this[kFinish]();
onStreamCloseResponse.call(stream);
else
stream.end();
}
Expand Down Expand Up @@ -649,18 +675,6 @@ class Http2ServerResponse extends Stream {
this[kStream].respond(headers, options);
}

[kFinish]() {
const stream = this[kStream];
const state = this[kState];
if (state.closed || stream.headRequest !== state.headRequest)
return;
state.closed = true;
this[kProxySocket] = null;
stream[kResponse] = undefined;
this.emit('finish');
this.emit('close');
}

// TODO doesn't support callbacks
writeContinue() {
const stream = this[kStream];
Expand Down

0 comments on commit d1b7825

Please sign in to comment.