Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for Unix domain sockets (UDS) #17

Closed
wants to merge 3 commits into from
Closed
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Add support for Unix domain sockets,
This change adds support for unix domain socket transport for using unix:// and udg:// for connections.
  • Loading branch information
RafaelKa committed Jun 2, 2015
commit 5267a138edfe85efeb4be0c365e2f674c340a009
20 changes: 16 additions & 4 deletions src/Server.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,26 @@ public function __construct(LoopInterface $loop)

public function listen($port, $host = '127.0.0.1')
{
if (strpos($host, ':') !== false) {
$localSocket = '';
if (filter_var($host, FILTER_VALIDATE_IP, FILTER_FLAG_IPV4)) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

composer.json should reflect that react requires ext-filter

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@staabm
This ext. is since PHP 5.2 enabled by default. http://php.net/manual/en/filter.installation.php
I see many methods from multiple PHP ext.s used in react and no entry in composer.json for them.
What is the difference by ext-filter?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which extensions does React use that aren't declared?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cboden f.x. PCRE (for preg_replace) is like Filter enabled per default to.

Is it maybe better to preg_match both instead of using filter?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ext. is since PHP 5.2 enabled by default

@RafaelKa you neverthelesss should declar it, because a lot of people - especially on production servers - use non-default builds (``--disable-all`) which are stripped down to the very least for performance and security reasons.

Which extensions does React use that aren't declared?

@cboden from within this code-window I would say e.g. ext-pcre

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you neverthelesss should declar it

I agree, all dependencies should explicitly be defined (and kept to a minimum for obvious reasons). Though in this case, have you checked PHP's installation instructions? (http://php.net/manual/en/pcre.installation.php and http://php.net/manual/en/filter.installation.php)

Which extensions does React use that aren't declared?

None that I'm aware of. It anybody happens to spot one, please file an issue or even better a PR in the relevant component :-)

$localSocket = 'tcp://' . $host . ':' . $port;
} elseif (filter_var($host, FILTER_VALIDATE_IP, FILTER_FLAG_IPV6)) {
// enclose IPv6 addresses in square brackets before appending port
$host = '[' . $host . ']';
$localSocket = 'tcp://[' . $host . ']:' . $port;
} elseif (preg_match('#^(unix|udg)://#', $host)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UDG sockets are based on datagrams instead of streams of bytes. They're already part of React's datagram component.

$localSocket = $host;
}

$this->master = @stream_socket_server("tcp://$host:$port", $errno, $errstr);
if (empty($localSocket)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check can probably be avoided by moving this to the else branch above?

throw new \UnexpectedValueException(
'"' . $host . '" does not match to a set of supported transports. ' .
'Supported transports are: IPv4, IPv6, unix:// and udg:// .'
, 1433253311);
}

$this->master = @stream_socket_server($localSocket, $errno, $errstr);
if (false === $this->master) {
$message = "Could not bind to tcp://$host:$port: $errstr";
$message = "Could not bind to $localSocket . Error: $errstr";
throw new ConnectionException($message, $errno);
}
stream_set_blocking($this->master, 0);
Expand Down