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

BIO_set_accept_name(): To accept from any interface, use * #21989

Closed

Conversation

levitte
Copy link
Member

@levitte levitte commented Sep 7, 2023

Using "*:{port}" is preferred to "[::]:{port}", because it won't break on IPv4-only machines. (NOTE: {port} is, of course, a port number or service name)

This fixes test failures in 79-test_http.t and 80-test_ssl_new.t on machines without IPv6.

Using "*:{port}" is preferred to "[::]:{port}", because it won't break on
IPv4-only machines.

This fixes test failures in 79-test_http.t and 80-test_ssl_new.t on machines
without IPv6.
@levitte levitte added branch: master Merge to master branch approval: review pending This pull request needs review by a committer triaged: bug The issue/pr is/fixes a bug tests: exempted The PR is exempt from requirements for testing labels Sep 7, 2023
@paulidale
Copy link
Contributor

Test failures are relevant.

Because apps/lib/http_server.c had a hard coded "[::]" for the accept host,
80-test_cmp_http.t assumed that it would always get a CMP server on an IPv6
address, and tested for that.

With the fix in apps/lib/http_server.c, that test was of course doomed to
fail.  Since CMP should be about IP version testing, 80-test_cmp_http.t is
adapted to allow the Mock server to accept connections on either IP version,
and the test for IPv6 is removed.
@levitte
Copy link
Member Author

levitte commented Sep 7, 2023

80-test_cmp_http.t did some bogus stuff. Fix commit added to make it a bit more flexible.

That test would have failed on any machine that doesn't have IPv6... the only reason I didn't see that is that for the one machine I have that only has IPv4, that test is disabled.

@levitte
Copy link
Member Author

levitte commented Sep 7, 2023

Pending on one og the buildbot workers... I've already alerted the owner of that worker.

@t8m t8m added this to the 3.2.0 milestone Sep 7, 2023
@paulidale paulidale added approval: done This pull request has the required number of approvals and removed approval: review pending This pull request needs review by a committer labels Sep 8, 2023
@levitte
Copy link
Member Author

levitte commented Sep 8, 2023

Merged

b0da24b BIO_set_accept_name(): To accept from any interface, use *
769e47e Fix 80-test_cmp_http.t to be more flexible regarding IP versions

@levitte levitte closed this Sep 8, 2023
@levitte levitte deleted the fix-failing-ipv6-tests-20230907 branch September 8, 2023 06:29
openssl-machine pushed a commit that referenced this pull request Sep 8, 2023
Using "*:{port}" is preferred to "[::]:{port}", because it won't break on
IPv4-only machines.

This fixes test failures in 79-test_http.t and 80-test_ssl_new.t on machines
without IPv6.

Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from #21989)
openssl-machine pushed a commit that referenced this pull request Sep 8, 2023
Because apps/lib/http_server.c had a hard coded "[::]" for the accept host,
80-test_cmp_http.t assumed that it would always get a CMP server on an IPv6
address, and tested for that.

With the fix in apps/lib/http_server.c, that test was of course doomed to
fail.  Since CMP should be about IP version testing, 80-test_cmp_http.t is
adapted to allow the Mock server to accept connections on either IP version,
and the test for IPv6 is removed.

Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from #21989)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approval: done This pull request has the required number of approvals branch: master Merge to master branch tests: exempted The PR is exempt from requirements for testing triaged: bug The issue/pr is/fixes a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants