Skip to content

Commit

Permalink
http: fix ClientRequest unhandled errors
Browse files Browse the repository at this point in the history
ClientRequest could someone cause an unhandled error
from socket.

Fixes: nodejs#36931
  • Loading branch information
ronag committed Jan 16, 2021
1 parent e39a3f8 commit 4fe8533
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 4 deletions.
8 changes: 5 additions & 3 deletions lib/_http_agent.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ const {
const { once } = require('internal/util');
const { validateNumber, validateOneOf } = require('internal/validators');

function nop() {}

const kOnKeylog = Symbol('onkeylog');
const kRequestOptions = Symbol('requestOptions');
const kRequestAsyncResource = Symbol('requestAsyncResource');
Expand Down Expand Up @@ -131,7 +133,7 @@ function Agent(options) {
// and could cause unhandled exception.

if (!socket.writable) {
socket.destroy();
socket.on('error', nop).destroy();
return;
}

Expand Down Expand Up @@ -159,7 +161,7 @@ function Agent(options) {
// the freeSockets pool, but only if we're allowed to do so.
const req = socket._httpMessage;
if (!req || !req.shouldKeepAlive || !this.keepAlive) {
socket.destroy();
socket.on('error', nop).destroy();
return;
}

Expand All @@ -173,7 +175,7 @@ function Agent(options) {
count > this.maxSockets ||
freeLen >= this.maxFreeSockets ||
!this.keepSocketAlive(socket)) {
socket.destroy();
socket.on('error', nop).destroy();
return;
}

Expand Down
2 changes: 1 addition & 1 deletion lib/_http_client.js
Original file line number Diff line number Diff line change
Expand Up @@ -801,7 +801,7 @@ function onSocketNT(req, socket, err) {
req.emit('close');
}

if (!err && req.agent) {
if (!err && req.agent && !socket.destroyed) {
socket?.emit('free');
} else if (socket) {
finished(socket.destroy(err || req[kError]), (er) => {
Expand Down
12 changes: 12 additions & 0 deletions test/parallel/test-http-client-abort3.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
'use strict';

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

const req = http.get('http://[2604:1380:45f1:3f00::1]:4002');

req.on('error', common.mustCall((err) => {
assert.strictEqual(err.code, 'EHOSTUNREACH');
}));
req.abort();

0 comments on commit 4fe8533

Please sign in to comment.