Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit048e0be

Browse files
authored
tls: ensure TLS Sockets are closed if the underlying wrap closes
This fixes a potential segfault, among various other likely-relatedissues, which all occur because TLSSockets were not informed if theirunderlying stream was closed in many cases.This also significantly modifies an existing TLS test. With this changein place, that test no longer works, as it tries to mess with internalsto trigger a race, and those internals are now cleaned up earlier. Thistest has been simplified to a more general TLS shutdown test.PR-URL:#49327Reviewed-By: Matteo Collina <matteo.collina@gmail.com>Reviewed-By: Debadree Chatterjee <debadree333@gmail.com>
1 parent92fb7dd commit048e0be

File tree

3 files changed

+91
-43
lines changed

3 files changed

+91
-43
lines changed

‎lib/_tls_wrap.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -704,6 +704,9 @@ TLSSocket.prototype._wrapHandle = function(wrap, handle, wrapHasActiveWriteFromP
704704
defineHandleReading(this,handle);
705705

706706
this.on('close',onSocketCloseDestroySSL);
707+
if(wrap){
708+
wrap.on('close',()=>this.destroy());
709+
}
707710

708711
returnres;
709712
};
Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
'use strict';
2+
3+
constcommon=require('../common');
4+
constfixtures=require('../common/fixtures');
5+
if(!common.hasCrypto)
6+
common.skip('missing crypto');
7+
constassert=require('assert');
8+
constnet=require('net');
9+
consth2=require('http2');
10+
11+
consttlsOptions={
12+
key:fixtures.readKey('agent1-key.pem'),
13+
cert:fixtures.readKey('agent1-cert.pem'),
14+
ALPNProtocols:['h2']
15+
};
16+
17+
// Create a net server that upgrades sockets to HTTP/2 manually, handles the
18+
// request, and then shuts down via a short socket timeout and a longer H2 session
19+
// timeout. This is an unconventional way to shut down a session (the underlying
20+
// socket closing first) but it should work - critically, it shouldn't segfault
21+
// (as it did until Node v20.5.1).
22+
23+
letserverRawSocket;
24+
letserverH2Session;
25+
26+
constnetServer=net.createServer((socket)=>{
27+
serverRawSocket=socket;
28+
h2Server.emit('connection',socket);
29+
});
30+
31+
consth2Server=h2.createSecureServer(tlsOptions,(req,res)=>{
32+
res.writeHead(200);
33+
res.end();
34+
});
35+
36+
h2Server.on('session',(session)=>{
37+
serverH2Session=session;
38+
});
39+
40+
netServer.listen(0,common.mustCall(()=>{
41+
constproxyClient=h2.connect(`https://localhost:${netServer.address().port}`,{
42+
rejectUnauthorized:false
43+
});
44+
45+
proxyClient.on('close',common.mustCall(()=>{
46+
netServer.close();
47+
}));
48+
49+
constreq=proxyClient.request({
50+
':method':'GET',
51+
':path':'/'
52+
});
53+
54+
req.on('response',common.mustCall((response)=>{
55+
assert.strictEqual(response[':status'],200);
56+
57+
// Asynchronously shut down the server's connections after the response,
58+
// but not in the order it typically expects:
59+
setTimeout(()=>{
60+
serverRawSocket.destroy();
61+
62+
setTimeout(()=>{
63+
serverH2Session.close();
64+
},10);
65+
},10);
66+
}));
67+
}));

‎test/parallel/test-tls-socket-close.js

Lines changed: 21 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -8,73 +8,51 @@ const tls = require('tls');
88
constnet=require('net');
99
constfixtures=require('../common/fixtures');
1010

11-
// Regression test for https://github.com/nodejs/node/issues/8074
12-
//
13-
// This test has a dependency on the order in which the TCP connection is made,
14-
// and TLS server handshake completes. It assumes those server side events occur
15-
// before the client side write callback, which is not guaranteed by the TLS
16-
// API. It usually passes with TLS1.3, but TLS1.3 didn't exist at the time the
17-
// bug existed.
18-
//
19-
// Pin the test to TLS1.2, since the test shouldn't be changed in a way that
20-
// doesn't trigger a segfault in Node.js 7.7.3:
21-
// https://github.com/nodejs/node/issues/13184#issuecomment-303700377
22-
tls.DEFAULT_MAX_VERSION='TLSv1.2';
23-
2411
constkey=fixtures.readKey('agent2-key.pem');
2512
constcert=fixtures.readKey('agent2-cert.pem');
2613

27-
lettlsSocket;
28-
// tls server
14+
letserverTlsSocket;
2915
consttlsServer=tls.createServer({ cert, key},(socket)=>{
30-
tlsSocket=socket;
31-
socket.on('error',common.mustCall((error)=>{
32-
assert.strictEqual(error.code,'EINVAL');
33-
tlsServer.close();
34-
netServer.close();
35-
}));
16+
serverTlsSocket=socket;
3617
});
3718

19+
// A plain net server, that manually passes connections to the TLS
20+
// server to be upgraded
3821
letnetSocket;
39-
// plain tcp server
4022
constnetServer=net.createServer((socket)=>{
41-
// If client wants to use tls
4223
tlsServer.emit('connection',socket);
4324

4425
netSocket=socket;
4526
}).listen(0,common.mustCall(function(){
4627
connectClient(netServer);
4728
}));
4829

30+
// A client that connects, sends one message, and closes the raw connection:
4931
functionconnectClient(server){
50-
consttlsConnection=tls.connect({
32+
constclientTlsSocket=tls.connect({
5133
host:'localhost',
5234
port:server.address().port,
5335
rejectUnauthorized:false
5436
});
5537

56-
tlsConnection.write('foo','utf8',common.mustCall(()=>{
38+
clientTlsSocket.write('foo','utf8',common.mustCall(()=>{
5739
assert(netSocket);
5840
netSocket.setTimeout(common.platformTimeout(10),common.mustCall(()=>{
59-
assert(tlsSocket);
60-
// This breaks if TLSSocket is already managing the socket:
41+
assert(serverTlsSocket);
42+
6143
netSocket.destroy();
62-
constinterval=setInterval(()=>{
63-
// Checking this way allows us to do the write at a time that causes a
64-
// segmentation fault (not always, but often) in Node.js 7.7.3 and
65-
// earlier. If we instead, for example, wait on the `close` event, then
66-
// it will not segmentation fault, which is what this test is all about.
67-
if(tlsSocket._handle._parent.bytesRead===0){
68-
tlsSocket.write('bar');
69-
clearInterval(interval);
70-
}
71-
},1);
44+
45+
setImmediate(()=>{
46+
assert.strictEqual(netSocket.destroyed,true);
47+
assert.strictEqual(clientTlsSocket.destroyed,true);
48+
49+
setImmediate(()=>{
50+
assert.strictEqual(serverTlsSocket.destroyed,true);
51+
52+
tlsServer.close();
53+
netServer.close();
54+
});
55+
});
7256
}));
7357
}));
74-
tlsConnection.on('error',(e)=>{
75-
// Tolerate the occasional ECONNRESET.
76-
// Ref: https://github.com/nodejs/node/issues/13184
77-
if(e.code!=='ECONNRESET')
78-
throwe;
79-
});
8058
}

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp