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

Return Connection object which exposes remote address #44

Closed
clue opened this issue Sep 7, 2015 · 4 comments · Fixed by #82
Closed

Return Connection object which exposes remote address #44

clue opened this issue Sep 7, 2015 · 4 comments · Fixed by #82

Comments

@clue
Copy link
Contributor

clue commented Sep 7, 2015

Each connector currently resolves with a Stream instance from the react/stream component. We should look into resolving with a Connection instance (which likely extends the Stream class) which should expose the remote address like this:

$connector->create('google.com', 80)->then(function (Connection $conn) {
    echo 'connected to ' . $conn->getRemoteAddress();
    $conn->close();
});

Note that the react/socket component already has a similar class (https://github.com/reactphp/socket/blob/master/src/Connection.php) which represents the server-side end of a connection.

@clue
Copy link
Contributor Author

clue commented Apr 20, 2016

See also reactphp/reactphp#199

@samuel-allan
Copy link

Mind if I try fixing this and submitting a PR?

@clue
Copy link
Contributor Author

clue commented Nov 27, 2016

I've started working on this a few months ago, finished the tests and code but didn't get to properly document this and the potential BC break. The other PRs were also certainly more pressuring back then, but this is now higher on my todo list again.

Other than that, not at all, go ahead 👍

@samuel-allan
Copy link

Nice! I will probably now start work on the other remote addr issue (clue-legacy/php-socks-server#5)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants