Repository navigation
Conversation
…ods for retrieving the remote and local pid.
|
What's the status on this? Was BC break label added as precaution, or is compatibility truly broken? If the issue is no longer setting the 'unix' public property to true, there's no reason that can't still be done in this PR. Let me know if there's anything I can do to help get this feature merged. |
|
Please let me know if there is something I can do to help with this PR :) |
clue
left a comment
clue
left a comment
There was a problem hiding this comment.
Thank you for working on this PR and giving us a better base to discuss what this feature could look like! I very much appreciate the effort!
I've added some technical remarks with the current change set below, but IMHO the main issue is not really about your patch but more about how we could integrate this feature in this library in the first place. Maybe we can continue this discussion in #150? 👍
| // If the remote pid has already been cached, return that value. | ||
| if ($this->remote_pid !== null) { | ||
| return $this->remote_pid; | ||
| } |
There was a problem hiding this comment.
What is the motivation for caching the PID here?
| } | ||
|
|
||
| // Get the PID of the remote side of the socket. | ||
| $pid = socket_get_option($socket, SOL_SOCKET, self::SO_PEERCRED); |
There was a problem hiding this comment.
What if ext-sockets is not available?
| return null; | ||
| } | ||
|
|
||
| $this->remote_pid = (int)$pid; |
There was a problem hiding this comment.
From the discussion in #150:
Interestingly
SO_PEERCREDsocket option should return aucredstructure withpid,uidandgid. Right now, PHP does not really support this constant and simply seems to return the first element only.
What if PHP starts supporting SO_PEERCRED? It's my understanding that this may break in a future PHP version?
| return $this->local_pid; | ||
| } | ||
|
|
||
| $pid = getmypid(); |
There was a problem hiding this comment.
What's the motivation for this method? Looks like this should not be part of the socket API?
| * | ||
| * @see Connection | ||
| */ | ||
| class UnixConnection extends Connection |
There was a problem hiding this comment.
Unlike the Connection, this class is currently not marked as @internal. Do we want this to become part of our public API?
| { | ||
| $loop = Factory::create(); | ||
|
|
||
| $server = new UnixServer($this->getRandomSocketUri(), $loop); |
There was a problem hiding this comment.
These tests should probably be skipped on platforms without UDS support (Windows)? Also, the socket file should be removed once the server is closed.
| * @see http://php.net/manual/en/sockets.constants.php | ||
| * @see http://php.net/manual/en/function.socket-get-option.php#101380 | ||
| */ | ||
| const SO_PEERCRED = 17; |
There was a problem hiding this comment.
This seems to be platform dependent (Linux only?). For example, FreeBSD (and Mac?) seems to use LOCAL_PEERCRED instead?
|
|
||
| ### UnixConnection | ||
| The `UnixConnection` is a specific implementation of a `ConnectionInterface` used to represent any | ||
| incoming and outgoing connection over a Unix domain socket (UDS). |
There was a problem hiding this comment.
This means that consumers need to depend on a concretion rather instead of an abstraction?
|
As much as I'd love to get this feature in, I'm having to close this for now as it hasn't received any input in a while and it's unlikely this will get traction any time soon. The feature request is still open in #150 and we'll look into this again in the not too far future 👍 If you feel this was closed prematurely or want to pick this up again, please let us know and we can reopen this. Thank you for your effort @gdejong, keep it coming! |
4 participants
Connections through a Unix Domain Socket will now be represented by a
UnixConnection(which extends a regularConnection). ThisUnixConnectioncontains the custom logic for parsing parsing the address and has the new ability to retrieve the local and remote PID.From the server side a client has no "remote address". The remote PID can for example be used to distinguish multiple clients.
See #150