Also expose port for each connection

[clue]

Currently, the Connection::getRemoteAddress() method only returns the IP address of the remote peer.

There's currently no way to access its TCP/IP port.

It's quite trivial to add a getRemotePort() method, but we should think twice about whether we should limit ourselves to TCP/IP here.

Other schemes (such as Unix or perhaps even raw sockets) do not have a concept of "ports" and may not necessarily build on top of IP.

As such, we should consider just returning the full remote peer address from the getRemoteAddress() method.

[clue]

See also reactphp/reactphp#199

[SamMousa]

Alternatively it would be great to at least not remove it from the address.

Are there any plans to add this to the interface at some point?
To properly support proxying one would want to be able to override this function to specify the port / address of the other end of the tunnel... In such a scenario manually calling stream_get_name() does not suffice.

I'm not sure what the downside is of adding a getter for the port that returns null when there is no port (ie in case of a Unix socket). Alternatively we could create a TcpConnectionInterface that has a port.

Is any development / discussion on this topic being held elsewhere?

[clue]

Are there any plans to add this to the interface at some point?

This ticket has received little interest so far. Care to change this an file a PR? :)

Alternatively it would be great to at least not remove it from the address.

I think this is what makes most sense, see also reactphp/reactphp#199 for more details.

[SamMousa]

@clue I've read reactphp/reactphp#199, the issue is that the longer we wait with making a choice the more libraries will break when we do decide to change it.

Just returning the result from stream_get_name is in my opinion not the best thing to do.
Of course it could be interesting to formalize addresses as well. (AddressInterface) but that might be overkill.

I'm definitely available to make a PR but let's first agree on the desired direction.
https://github.com/SAM-IT/react-socket

[clue]

Just returning the result from stream_get_name is in my opinion not the best thing to do.

Care to elaborate what your opinion is based on?

Of course it could be interesting to formalize addresses as well. (AddressInterface) but that might be overkill.

I agree that this would be the cleanest and totally overkill solution :)

[SamMousa]

Well the whole point of implementing a Connection is abstraction. Returning a string like that doesn't feel right to me.

[clue]

For the most part, Connection is an abstraction of a duplex stream (plus a single accessor method for the remote address). Given the little interest in this ticket so far and looking at existing libraries building on top of this library, the remote address appears to used way less than the stream abstraction (from what I can tell).

Technically, the "port" is part of the "address" (for TCP/IP anyway).

We could introduce a second accessor method for the remote port and then rename the first method to make it obvious it does not include the port. However, this would be a leaky abstraction as we've now leaked the detail that we focus on TCP/IP connections and other protocols do not know the concept of a "port".

Alternatively, we could change the single accessor method to return the full address. The string format returned by the underlying stream_get_name() would probably be the simplest solution here. This is in line with how URIs are represented throughout React's components (or planned for the future as per reactphp/reactphp#199) and many other third-party libraries.

I agree that having an interface (kind of like PSR-7's UriInterface) sounds tempting here. If we want to implement this throughout React's components we'd have to add dependencies to a shared component. For consistency, we'd likely also type-hint against this and thus require users to create new instances before establishing connections to URIs or setting up listening addresses.

As such, I'm in favor of using a single method to access the URI and I'm not really convinced an additional interface is actually worth the hassle. I'm curious what others think about this?

[SamMousa]

That sounds ok to me, and I agree that doing a true abstraction requires too much overhead and a leaky abstraction is not what we want; so that leaves getRemoteAddress.