EcmaTC53 / spec

Ecma TC53 spec work
23 stars 9 forks source link

fix: tcp listener socket port argument is optional #43

Closed HipsterBrown closed 11 months ago

HipsterBrown commented 11 months ago

The description of the TCP listener socket (Listener IO class) specifies that the port property of the constructor options object is optional. However, the algorithm section specifies the port property is required.

The Moddable implementation follows the optional property logic and defaults the value to 0.

If we reference other well-known implementations:

If port is omitted or is 0, the operating system will assign an arbitrary unused port

This appears to be the case in lwIP TCP as well.

phoddie commented 11 months ago

There are inconsistencies here. If the port is optional, one will be assigned. I don't see a way for the caller to know which port was assigned though. There is a model for that with the remote port property.

HipsterBrown commented 11 months ago

@phoddie In Node.js, the selected port is available via the address() method on the net.Server instance. Possibly we should discuss adding this property to the Listener instance? This appears to be available from the lwIP tcp_pcb struct for implementations built upon that stack.

phoddie commented 11 months ago

@HipsterBrown - Yes, if we are going to allow for a port of 0 we should add a way to get the assigned port. My feeling is that we should follow the pattern we have for the remote port and remote address properties on the TCP socket, but I'm open to other approaches (e.g. address()). Easy topic for our January call.

We can merge this PR as-is. It reflects reality. Are you good with that?

HipsterBrown commented 11 months ago

My feeling is that we should follow the pattern we have for the remote port and remote address properties on the TCP socket

I was just looking at that as well 😄 That makes sense to me

Thank you for merging this. Looking forward discussing it next year.