Skip to content

Iterate through all DNS entries in connect CLI#2906

Merged
geedo0 merged 1 commit intoaws:mainfrom
geedo0:main
Dec 15, 2025
Merged

Iterate through all DNS entries in connect CLI#2906
geedo0 merged 1 commit intoaws:mainfrom
geedo0:main

Conversation

@geedo0
Copy link
Copy Markdown
Contributor

@geedo0 geedo0 commented Dec 15, 2025

Issues:

N/A

Description of changes:

The openssl sclient -connect CLI command would only attempt to connect to the first resolved DNS entry. Failing that,
it considers the entire connection attempt failed. This change updates the command to iterate through the linked list of
entries until it can successfully connect. This is in-line with upstream OpenSSL.

It's particularly useful in cases where the first address resolved is either unavailable or otherwise unusable. This can
be common when trying to resolve an address such as localhost and the OS returns the ipv6 loopback address before the
ipv4 version and your web server is only listening with ipv4.

Call-outs:

I did not exhaustively consider other uses of this function beyond this specific CLI tool. A glance check of the references suggests low usage and it's still the right change to make overall.

Testing:

  • Tested this in a build container which resolves localhost as [::1, 127.0.0.1] with a web server that listens on ipv4 and asserted that the CLI tool can connect successfully to that web server.

  • CI

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.

The `openssl sclient -connect` CLI command would only attempt to connect to the first resolved DNS entry. Failing that,
it considers the entire connection attempt failed. This change updates the command to iterate through the linked list of
entries until it can successfully connect. This is in-line with upstream [OpenSSL](https://github.com/openssl/openssl/blob/OpenSSL_1_1_1-stable/apps/s_socket.c#L93).

It's particularly useful in cases where the first address resolved is either unavailable or otherwise unusable. This can
be common when trying to resolve an address such as `localhost` and the OS returns the ipv6 loopback address before the
ipv4 version and your web server is only listening with ipv4.
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 44.00000% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.14%. Comparing base (c23b2ae) to head (a51fa61).

Files with missing lines Patch % Lines
tool/transport_common.cc 44.00% 12 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2906      +/-   ##
==========================================
- Coverage   78.15%   78.14%   -0.01%     
==========================================
  Files         687      687              
  Lines      118579   118577       -2     
  Branches    16671    16674       +3     
==========================================
- Hits        92671    92665       -6     
- Misses      25019    25023       +4     
  Partials      889      889              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@geedo0 geedo0 marked this pull request as ready for review December 15, 2025 18:17
@geedo0 geedo0 requested a review from a team as a code owner December 15, 2025 18:17
@geedo0 geedo0 enabled auto-merge (squash) December 15, 2025 18:18
@geedo0 geedo0 merged commit 5a08a98 into aws:main Dec 15, 2025
467 of 471 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants