Skip to content
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

Http2Session emits 'connect' event while socket is still connecting #49860

Open
chrmarti opened this issue Sep 25, 2023 · 2 comments
Open

Http2Session emits 'connect' event while socket is still connecting #49860

chrmarti opened this issue Sep 25, 2023 · 2 comments
Labels
http2 Issues or PRs related to the http2 subsystem.

Comments

@chrmarti
Copy link

Version

Tested with v20.7.0 and v18.17.1

Platform

Probably platform independent. Darwin MacBook-Pro-M1-Max.local 22.6.0 Darwin Kernel Version 22.6.0: Wed Jul 5 22:22:05 PDT 2023; root:xnu-8796.141.3~6/RELEASE_ARM64_T6000 arm64

Subsystem

http2

What steps will reproduce the bug?

When createConnection returns a socket that has connecting === true but no _handle set, the Http2Session constructor will wrap the socket with a JSStreamSocket and this wrapper will not reflect that the underlying socket is still connecting. The session will then emit a 'connect' event while the socket is still connecting.

Code demonstrating the issue when connecting to a port that is not being listened on:

const net = require('net');
const http2 = require('http2');

const session = http2.connect('http://[::1]:12345', {
	createConnection: () => {
		const socket = new net.Socket();
		socket.connecting = true;
		// socket._handle = { isStreamBase: true }; // Uncomment to confirm _handle makes a difference.
		socket.on('error', err => {
			console.error('socket:', err);
		});
		socket.on('connect', () => {
			console.log('socket: connect');
		});
		socket.on('close', () => {
			console.log('socket: close');
		});
		
		setTimeout(() => {
			// socket._handle = undefined; // Uncomment to confirm _handle makes a difference.
			socket.connect(12345, '::1');
		}, 100);

		return socket;
	}
});

session.on('error', err => {
	console.error('http2 session:', err);
});
session.on('connect', () => {
	console.log('http2 session: connect');
});
session.on('close', () => {
	console.log('http2 session: close');
});

The output:

http2 session: connect
socket: Error: connect ECONNREFUSED ::1:12345
    at TCPConnectWrap.afterConnect [as oncomplete] (node:net:1595:16) {
  errno: -61,
  code: 'ECONNREFUSED',
  syscall: 'connect',
  address: '::1',
  port: 12345
}
socket: close
http2 session: Error: connect ECONNREFUSED ::1:12345
    at TCPConnectWrap.afterConnect [as oncomplete] (node:net:1595:16) {
  errno: -61,
  code: 'ECONNREFUSED',
  syscall: 'connect',
  address: '::1',
  port: 12345
}
http2 session: close

The 'connect' event is unexpected.

Setting the _handle as the commented 2 lines of code suggest can trick the constructor of Http2Session into not wrapping the socket and wait for the socket to emit a 'connect' event before emitting 'connect' itself. This is provided only illustration, setting _handle this way is not expected to be a valid workaround.

Code pointers:

  • Http2Sesssion wraps the socket in a JSStreamSocket because the _handle is undefined:
    if (!socket._handle || !socket._handle.isStreamBase) {
  • The wrapper does not have connecting === true and the check of Http2Session whether the socket (now the wrapper) is still connecting does not trigger:
    if (socket.connecting || socket.secureConnecting) {

How often does it reproduce? Is there a required condition?

Always reproduces with the above sample code.

What is the expected behavior? Why is that the expected behavior?

Http2Session should only emit a 'connect' event once the underlying socket is connected.

What do you see instead?

Http2Session emits 'connect' event while socket is still connecting.

Additional information

No response

@darker16
Copy link

ဗားရှင်း

v20.7.0 နှင့် v18.17.1 ဖြင့် စမ်းသပ်ထားသည်။

ပလပ်ဖောင်း

ဒီဘက်က ပလက်ဖောင်း လွတ်လပ်တယ်။ Darwin MacBook-Pro-M1-Max.local 22.6.0 Darwin Kernel ဗားရှင်း 22.6.0: Wed Jul 5 22:22:05 PDT 2023; အမြစ်-xnu-8796.141.3~6/RELEASE_ARM64_T6000 arm64

စနစ်ခွဲ

http2

ဘယ်အဆင့်တွေက bug ကိုပြန်ထုတ်ပေးမလဲ။

သတ်မှတ်ထားခြင်း မရှိသော createConnectionsocket ကို ပြန်ပေး သောအခါ ၊ Http2Session တည်ဆောက်သူသည် socket ကို a ဖြင့် ပတ်မည်ဖြစ်ပြီး ၊ အရင်းခံ socket သည် ချိတ်ဆက်နေဆဲဖြစ်ကြောင်း ဤ wrapper က ထင်ဟပ်မည်မဟုတ်ပါ။ ထို့နောက် ဆက်ရှင်သည် socket ချိတ်ဆက်နေချိန်တွင် 'ချိတ်ဆက်ခြင်း' အစီအစဉ်ကို ထုတ်လွှတ်ပါမည်။connecting === true``_handle``JSStreamSocket

နားမထောင်သော ဆိပ်ကမ်းသို့ ချိတ်ဆက်ရာတွင် ပြဿနာကို ပြသသည့် ကုဒ်-

const net = require('net');
const http2 = require('http2');

const session = http2.connect('http://[::1]:12345', {
	createConnection: () => {
		const socket = new net.Socket();
		socket.connecting = true;
		// socket._handle = { isStreamBase: true }; // Uncomment to confirm _handle makes a difference.
		socket.on('error', err => {
			console.error('socket:', err);
		});
		socket.on('connect', () => {
			console.log('socket: connect');
		});
		socket.on('close', () => {
			console.log('socket: close');
		});
		
		setTimeout(() => {
			// socket._handle = undefined; // Uncomment to confirm _handle makes a difference.
			socket.connect(12345, '::1');
		}, 100);

		return socket;
	}
});

session.on('error', err => {
	console.error('http2 session:', err);
});
session.on('connect', () => {
	console.log('http2 session: connect');
});
session.on('close', () => {
	console.log('http2 session: close');
});

အထွက်-

http2 session: connect
socket: Error: connect ECONNREFUSED ::1:12345
    at TCPConnectWrap.afterConnect [as oncomplete] (node:net:1595:16) {
  errno: -61,
  code: 'ECONNREFUSED',
  syscall: 'connect',
  address: '::1',
  port: 12345
}
socket: close
http2 session: Error: connect ECONNREFUSED ::1:12345
    at TCPConnectWrap.afterConnect [as oncomplete] (node:net:1595:16) {
  errno: -61,
  code: 'ECONNREFUSED',
  syscall: 'connect',
  address: '::1',
  port: 12345
}
http2 session: close

'ချိတ်ဆက်' ဖြစ်ရပ်သည် မျှော်လင့်မထားပါ။

မှတ်ချက်ပေးထားသော ကုဒ် 2 ကြောင်းမှ အကြံပြုထားသည့်အတိုင်း သတ်မှတ်ခြင်း _handleသည် Http2Session ၏ constructor ကို socket ကို မခြုံမိစေရန် လှည့်ဖြားနိုင်ပြီး socket သည် 'connect' ဖြစ်ရပ်ကို မထုတ်မီ 'connect' အစီအစဉ်ကို ထုတ်လွှတ်ရန်အတွက် စောင့်နိုင်သည်။ ဤနည်းလမ်းသည် _handleမှန်ကန်သော ဖြေရှင်းနည်းတစ်ခုဟု မမျှော်လင့်ထားပေ။

ကုဒ်ညွှန်ပြချက်များ-

  • Http2Session သည် သတ်မှတ်မထားသော ဖြစ်သောကြောင့် JSStreamSocket တွင် socket ကို ခြုံထားသည် _handle-
    if (!socket._handle || !socket._handle.isStreamBase) {
  • ထုပ်ပိုးခြင်းတွင် မရှိပါ connecting === trueနှင့် Http2Session ၏ စစ်ဆေးခြင်း socket (ယခု wrapper) သည် ချိတ်ဆက်နေသေးခြင်း ရှိ၊
    if (socket.connecting || socket.secureConnecting) {

ဘယ်နှစ်ကြိမ် မျိုးပွားသလဲ။ လိုအပ်သောအခြေအနေရှိပါသလား။

အထက်ဖော်ပြပါ နမူနာကုဒ်ဖြင့် အမြဲပြန်ထုတ်ပေးသည်။

မျှော်မှန်းထားသည့် အပြုအမူမှာ အဘယ်နည်း။ အဘယ့်ကြောင့်ဆိုသော် ၎င်းသည် မျှော်လင့်ထားသော အပြုအမူဖြစ်သနည်း။

Http2Session သည် အရင်းခံ socket ချိတ်ဆက်ပြီးသည်နှင့် 'ချိတ်ဆက်' ဖြစ်ရပ်ကိုသာ ထုတ်လွှတ်သင့်သည်။

အစား ဘာကိုမြင်လဲ။

Http2Session သည် socket ချိတ်ဆက်နေချိန်တွင် 'ချိတ်ဆက်ခြင်း' အစီအစဉ်ကို ထုတ်လွှတ်ပါသည်။

နောက်ထပ်အချက်အလက်များ

တုံ့ပြန်မှုမရှိပါ။

@mertcanaltin mertcanaltin added the http2 Issues or PRs related to the http2 subsystem. label Sep 30, 2023
@Tushar-Gupta11
Copy link

Hey!! I am quite new to the code base, what i didnt understand was why in the first place is node not assuring that the socket is connected, would it be plausible if the condition is changed from socket.connecting to checking if the socket is connected or not already. Asking because since the issue is with jsstreamsocket connection only, would it be right to change the condition all over or should i search for a way to return from jsstreamsocket itself that the socket has connected?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
http2 Issues or PRs related to the http2 subsystem.
Projects
None yet
Development

No branches or pull requests

4 participants