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

Run stream_server_reneg_limit.phpt on Windows, too #17193

Closed
wants to merge 1 commit into from

Conversation

cmb69
Copy link
Member

@cmb69 cmb69 commented Dec 17, 2024

As is, the client code may block on Windows when trying to read STDERR; if that happens, neither the openssl nor the php processes would be terminated, causing further issues for the test suite.

We fix this by using stream_select() and only read if there is data available. This may cause some delay until we hit the timeout, but it might be necessary to "empty" STDERR.


Tackling #16086 piecemeal might be a better idea.

@cmb69
Copy link
Member Author

cmb69 commented Dec 17, 2024

Might work now on Windows, but need to wait for #17180.

As is, the client code may block on Windows when trying to read STDERR;
if that happens, neither the openssl nor the php processes would be
terminated, causing further issues for the test suite.

We fix this by using `stream_select()` and only read if there is data
available.  This may cause some delay until we hit the timeout, but it
might be necessary to "empty" STDERR.
@cmb69 cmb69 force-pushed the cmb/stream_server_reneg_limit.phpt branch from caf3913 to f8a9b81 Compare January 27, 2025 19:37
@cmb69
Copy link
Member Author

cmb69 commented Jan 27, 2025

[…], but need to wait for #17180.

Nope, the issues are not caused by port collisions. I have no idea what's the cause, though.

Might not make much sense to spend further time on this, given that renegotiation is a thing of the past (not supported by TLS 1.3), so I'm closing this PR.

@cmb69 cmb69 closed this Jan 27, 2025
@cmb69 cmb69 deleted the cmb/stream_server_reneg_limit.phpt branch January 27, 2025 23:32
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.

1 participant