nodejs / node

Node.js JavaScript runtime ✨🐢🚀✨
https://nodejs.org
Other
107.82k stars 29.71k forks source link

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

Open chrmarti opened 1 year ago

chrmarti commented 1 year ago

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:

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 commented 1 year ago

ဗားရှင်း

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 သည် အရင်းခံ socket ချိတ်ဆက်ပြီးသည်နှင့် 'ချိတ်ဆက်' ဖြစ်ရပ်ကိုသာ ထုတ်လွှတ်သင့်သည်။

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

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

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

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

Tushar-Gupta11 commented 1 year ago

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?