-
-
Notifications
You must be signed in to change notification settings - Fork 722
Description
Calling Socket\Server::listen($port, $host) with a hostname instead of an IPv4 address is a blocking operation.
Example:
$server = new Socket\Server($loop);
$server->listen(1234, 'example.com');Giving a hostname will result in resolving it via DNS (blocking).
Possible solutions include:
- Reject hostnames altogether and only accept IP addresses. Easy temporary solution, but listening on hostnames is a convenient feature, think of listening on
localhost:1234 - Accept this as a known limitation and block. Not likely that we want to see something like this in an async library though
- Async resolving via React\Dns component: Requires passing in a Resolver instance somewhere and will have to change the
listen()method to return a Promise instead. Should be pretty straight forward, but changing the API in such a subtle way is likely going to break existing code - Create new
Socket\Factoryclass that does all the work, accept an optional Resolver instance to resolve any hostnames before passing it to alisten()call.
Personally, I'd go for suggestion 4 as we have a number of outstanding issues with Socket\Server anyway (UNIX sockets, SSL sockets, no IPv6 support, API consistency, etc.). I've implemented a very similar factory for clue/socket-react here and here and for clue/socket-raw here. It's been working out quite good (feel free to take a look at each projects' examples) and I'd love to see something similar in react.
In the future this could possibly also allow merging SocketClient and Socket into a single package as they share quite a bit of code and it could allow us to call
Factory::createServer('localhost:1234')->then(function (Server $server) { });
Factory::createClient('localhost:1234')->then(function (Connection $client) {
$client->end('hello');
});This provides a very consistent API, is very similar to how stream_socket_server() and stream_socket_client() work and could easily be extended to support IPv6, SSL and UNIX domain sockets.