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

Fix GH-16955: Use ephemeral ports for OpenSSL server client tests #17180

Closed

Conversation

bukka
Copy link
Member

@bukka bukka commented Dec 16, 2024

This introduces usage of ephemeral port for majority of OpenSSL test.

The only tests that need more work are following:

  • stream_server_reneg_limit.phpt - quite manual reneg test - might need some update anyway
  • bug77390 - proxy scenario - it needs more work in empheral port support
  • ext/standard/tests/streams/stream_context_tcp_nodelay_server.phpt - this is a bit non standard test - need to figure out how to best address it there

@bukka bukka changed the base branch from master to PHP-8.1 December 16, 2024 19:29
@cmb69 cmb69 linked an issue Dec 16, 2024 that may be closed by this pull request
@cmb69
Copy link
Member

cmb69 commented Dec 16, 2024

The only tests that need more work are following:

If there are only three left, I'm fine with manually choosing hard-coded port numbers, or adding a conflict marker.

@bukka bukka force-pushed the openssl_test_server_client_empheral_port branch from e2422cb to f4c2104 Compare December 17, 2024 12:38
@bukka
Copy link
Member Author

bukka commented Dec 17, 2024

I rewrote that stream_context_tcp_nodelay_server.phpt which was just wrong and then added support for multiple workers which fixes bug77390.phpt . It's a bit more complex now but it works. The only one left is stream_server_reneg_limit.phpt but that's skipped on Windows. It might need some rewrite or we will just eventually drop it (reneg is not really used and not even supported in TLS 1.3). I will leave it for now and if there are any issues with it, I will look into it.

@bukka
Copy link
Member Author

bukka commented Dec 17, 2024

hmm that stream one no longer works on Windows. Will check it out next week. Also will need to check if there are more tests in later versions.

@cmb69
Copy link
Member

cmb69 commented Dec 17, 2024

The only one left is stream_server_reneg_limit.phpt but that's skipped on Windows.

See #17193.

that stream one no longer works on Windows.

I'll have a look as soon as possible.

@cmb69
Copy link
Member

cmb69 commented Dec 17, 2024

I have not been able to reproduce failing of stream_context_tcp_nodelay_server.phpt locally, so I've triggered the Windows job again to check whether this is flaky.

Also will need to check if there are more tests in later versions.

Indeed; in master there are also gh11418.phpt and bug51056.phpt (and maybe more).

@cmb69
Copy link
Member

cmb69 commented Dec 18, 2024

stream_context_tcp_nodelay_server.phpt failed again for the same reason (no output). Might be a crash in the server code, or due to writing to STDERR (could use fwrite() instead of fprintf() although unlikely to cause issues), or whatever. I still haven't been able to reproduce locally.

@bukka bukka changed the title Fix GH-16955: Use empheral ports for OpenSSL server client tests Fix GH-16955: Use ephemeral ports for OpenSSL server client tests Dec 22, 2024
@bukka bukka force-pushed the openssl_test_server_client_empheral_port branch 3 times, most recently from fdbaf8c to 89534a0 Compare December 31, 2024 11:47
@bukka bukka force-pushed the openssl_test_server_client_empheral_port branch from 0edf4fd to deb1498 Compare December 31, 2024 13:24
@bukka bukka closed this in b873176 Dec 31, 2024
@bukka
Copy link
Member Author

bukka commented Dec 31, 2024

@cmb69 So I did a bit of debugging and in this build https://github.com/php/php-src/actions/runs/12559675194/job/35015813633 with extra debug output, it gave me:

     client start: 127.0.0.1:%d
     server-accepted
003+ 
004+ Fatal error: Uncaught Error: Call to undefined function socket_import_stream() in D:\a\php-src\php-src\ext\openssl\tests\ServerClientTestCase.inc(124) : eval()'d code:15
005+ Stack trace:
006+ #0 D:\a\php-src\php-src\ext\openssl\tests\ServerClientTestCase.inc(124): eval()
003- server-imported
004- si:delay
005- conn-imported
006- server-delay:conn-nodelay

I decided to skip it for now on Windows so I can get it merged as I need those changes for another thing (well I don't want to port that thing later) that I'm looking to and created #17308 which debug output. Not sure at the moment why socket_import_stream is not present but might be related to how the worker is created on Win:

if (defined("PHP_WINDOWS_VERSION_MAJOR")) {
$ini = php_ini_loaded_file();
$cmd = sprintf(
'%s %s "%s" %s',
PHP_BINARY, $ini ? "-n -c $ini" : "",
__FILE__,
WORKER_ARGV_VALUE
);
. Please comment to that new PR if you have any idea - feel free to also push the changes into it if you want.

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.

openssl ServerClientTestCase tests cases use conflicting ports
3 participants