Skip to content

http: check "parser" existence in various conditions for http client #26486

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
169 changes: 87 additions & 82 deletions lib/_http_client.js
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,7 @@ function socketCloseListener() {
res.emit('close');
}
} else {
if (!req.socket._hadError) {
if (parser && !req.socket._hadError) {
// This socket error fired before we started to
// receive a response. The error needs to
// fire on the request.
Expand Down Expand Up @@ -424,7 +424,7 @@ function socketOnEnd() {
var req = this._httpMessage;
var parser = this.parser;

if (!req.res && !req.socket._hadError) {
if (parser && !req.res && !req.socket._hadError) {
// If we don't have a response then we know that the socket
// ended prematurely and we need to emit an error on the request.
req.socket._hadError = true;
Expand All @@ -442,57 +442,59 @@ function socketOnData(d) {
var req = this._httpMessage;
var parser = this.parser;

assert(parser && parser.socket === socket);

var ret = parser.execute(d);
if (ret instanceof Error) {
debug('parse error', ret);
freeParser(parser, req, socket);
socket.destroy();
req.socket._hadError = true;
req.emit('error', ret);
} else if (parser.incoming && parser.incoming.upgrade) {
// Upgrade (if status code 101) or CONNECT
var bytesParsed = ret;
var res = parser.incoming;
req.res = res;

socket.removeListener('data', socketOnData);
socket.removeListener('end', socketOnEnd);
socket.removeListener('drain', ondrain);
parser.finish();
freeParser(parser, req, socket);

var bodyHead = d.slice(bytesParsed, d.length);

var eventName = req.method === 'CONNECT' ? 'connect' : 'upgrade';
if (req.listenerCount(eventName) > 0) {
req.upgradeOrConnect = true;

// detach the socket
socket.emit('agentRemove');
socket.removeListener('close', socketCloseListener);
socket.removeListener('error', socketErrorListener);

socket._httpMessage = null;
socket.readableFlowing = null;
if (parser) {
assert(parser.socket === socket);

req.emit(eventName, res, socket, bodyHead);
req.emit('close');
} else {
// Requested Upgrade or used CONNECT method, but have no handler.
var ret = parser.execute(d);
if (ret instanceof Error) {
debug('parse error', ret);
freeParser(parser, req, socket);
socket.destroy();
req.socket._hadError = true;
req.emit('error', ret);
} else if (parser.incoming && parser.incoming.upgrade) {
// Upgrade (if status code 101) or CONNECT
var bytesParsed = ret;
var res = parser.incoming;
req.res = res;

socket.removeListener('data', socketOnData);
socket.removeListener('end', socketOnEnd);
socket.removeListener('drain', ondrain);
parser.finish();
freeParser(parser, req, socket);

var bodyHead = d.slice(bytesParsed, d.length);

var eventName = req.method === 'CONNECT' ? 'connect' : 'upgrade';
if (req.listenerCount(eventName) > 0) {
req.upgradeOrConnect = true;

// detach the socket
socket.emit('agentRemove');
socket.removeListener('close', socketCloseListener);
socket.removeListener('error', socketErrorListener);

socket._httpMessage = null;
socket.readableFlowing = null;

req.emit(eventName, res, socket, bodyHead);
req.emit('close');
} else {
// Requested Upgrade or used CONNECT method, but have no handler.
socket.destroy();
}
} else if (parser.incoming && parser.incoming.complete &&
// When the status code is informational (100, 102-199),
// the server will send a final response after this client
// sends a request body, so we must not free the parser.
// 101 (Switching Protocols) and all other status codes
// should be processed normally.
!statusIsInformational(parser.incoming.statusCode)) {
socket.removeListener('data', socketOnData);
socket.removeListener('end', socketOnEnd);
freeParser(parser, req, socket);
}
} else if (parser.incoming && parser.incoming.complete &&
// When the status code is informational (100, 102-199),
// the server will send a final response after this client
// sends a request body, so we must not free the parser.
// 101 (Switching Protocols) and all other status codes
// should be processed normally.
!statusIsInformational(parser.incoming.statusCode)) {
socket.removeListener('data', socketOnData);
socket.removeListener('end', socketOnEnd);
freeParser(parser, req, socket);
}
}

Expand Down Expand Up @@ -628,38 +630,41 @@ function emitFreeNT(socket) {

function tickOnSocket(req, socket) {
var parser = parsers.alloc();
req.socket = socket;
req.connection = socket;
parser.reinitialize(HTTPParser.RESPONSE, parser[is_reused_symbol]);
parser.socket = socket;
parser.outgoing = req;
req.parser = parser;

socket.parser = parser;
socket._httpMessage = req;

// Setup "drain" propagation.
httpSocketSetup(socket);

// Propagate headers limit from request object to parser
if (typeof req.maxHeadersCount === 'number') {
parser.maxHeaderPairs = req.maxHeadersCount << 1;
}

parser.onIncoming = parserOnIncomingClient;
socket.removeListener('error', freeSocketErrorListener);
socket.on('error', socketErrorListener);
socket.on('data', socketOnData);
socket.on('end', socketOnEnd);
socket.on('close', socketCloseListener);

if (
req.timeout !== undefined ||
(req.agent && req.agent.options && req.agent.options.timeout)
) {
listenSocketTimeout(req);
}
req.emit('socket', socket);
if (parser) {
req.socket = socket;
req.connection = socket;
parser.reinitialize(HTTPParser.RESPONSE, parser[is_reused_symbol]);
parser.socket = socket;
parser.outgoing = req;

req.parser = parser;

socket.parser = parser;
socket._httpMessage = req;

// Setup "drain" propagation.
httpSocketSetup(socket);

// Propagate headers limit from request object to parser
if (typeof req.maxHeadersCount === 'number') {
parser.maxHeaderPairs = req.maxHeadersCount << 1;
}

parser.onIncoming = parserOnIncomingClient;
socket.removeListener('error', freeSocketErrorListener);
socket.on('error', socketErrorListener);
socket.on('data', socketOnData);
socket.on('end', socketOnEnd);
socket.on('close', socketCloseListener);

if (
req.timeout !== undefined ||
(req.agent && req.agent.options && req.agent.options.timeout)
) {
listenSocketTimeout(req);
}
req.emit('socket', socket);
}
}

function listenSocketTimeout(req) {
Expand Down
24 changes: 24 additions & 0 deletions test/parallel/test-http-client-delete-parser.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
'use strict';

const common = require('../common');

const http = require('http');

const server = http.createServer(common.mustCall((req, res) => {
res.writeHead(200, { 'Content-Type': 'text/plain' });
res.write('okay', common.mustCall(() => {
delete req.socket.parser;
}));
res.end();
}));

server.listen(1337, '127.0.0.1');
server.unref();

const req = http.request({
port: 1337,
host: '127.0.0.1',
method: 'GET',
});

req.end();