Skip to content

Conversation

seliver
Copy link
Contributor

@seliver seliver commented Mar 31, 2018

The IPv6 IP of a socket is provided by the inet_ntop as a string,
but this function doesn't enclose the ip in brackets. This patch
adds them in the php_network_populate_name_from_sockaddr function.

The IPv6 IP of a socket is provided by the inet_ntop as a string,
but this function doesn't enclose the ip in brackets. This patch
adds them in the php_network_populate_name_from_sockaddr function.
@seliver
Copy link
Contributor Author

seliver commented Mar 31, 2018

I tried adding the pull request to the bug tracking system, but unfortunately it didn't work.

@carusogabriel
Copy link
Contributor

@seliver This was already reported at #76079. I'll look into that tonight 👍

echo stream_socket_get_name($server, false).PHP_EOL;
$server = stream_socket_server("tcp://127.0.0.1:1337/");
echo stream_socket_get_name($server, false);
--EXPECTF--
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing ?>.

Also, please use EXPECT, is simpler than EXPECTF 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Added PHP close tag and using expect 
instead of expectf
@seliver
Copy link
Contributor Author

seliver commented Apr 9, 2018

Is there something else that needs to be done on my side?

@nikic
Copy link
Member

nikic commented Apr 13, 2018

@seliver The Travis build is failing with:

========DIFF========
001+ Warning: stream_socket_server(): unable to connect to tcp://[::1]:1337/ (Cannot assign requested address) in /home/travis/build/php/php-src/tests/output/bug76136.php on line 2
002+ 
003+ Warning: stream_socket_get_name() expects parameter 1 to be resource, boolean given in /home/travis/build/php/php-src/tests/output/bug76136.php on line 3
004+ 
001- [::1]:1337
========DONE========
FAIL Bug #76136: stream_socket_get_name should enclose IPv6 in brackets [tests/output/bug76136.phpt] 

I assume that means that IPv6 is not supported? This probably needs a SKIPIF check.

Otherwise the change looks reasonable to me, but I'm not sure about BC impact etc. @bwoebi Can you check this? Which branch should it land on?

@nikic
Copy link
Member

nikic commented Jul 5, 2018

Ping @bwoebi @kelunik on BC impact. Can we do this and if so for 7.1 or 7.3 only?

@kelunik
Copy link
Member

kelunik commented Jul 6, 2018

7.3 only, because not every workaround might account for the bug being fixed. We and ReactPHP do, but others might not.

@nikic
Copy link
Member

nikic commented Jul 6, 2018

Okay, let's go for PHP 7.3 then. @seliver, can you please update the test to skip if ipv6 is not supported? This will need a SKIPIF section with a check like this: https://github.com/php/php-src/blob/master/sapi/fpm/tests/tester.inc#L253

@nikic
Copy link
Member

nikic commented Jul 7, 2018

Merged as 9501304 with test adjustment.

@nikic nikic closed this Jul 7, 2018
@seliver seliver deleted the PHP-7.1 branch January 23, 2019 09:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants