Skip to content

https-eyeballing async#21267

Closed
icing wants to merge 4 commits into
curl:masterfrom
icing:https-eyeballing-async
Closed

https-eyeballing async#21267
icing wants to merge 4 commits into
curl:masterfrom
icing:https-eyeballing-async

Conversation

@icing
Copy link
Copy Markdown
Contributor

@icing icing commented Apr 8, 2026

Make cf-https-connect work async correctly:

  • only start first baller when at least one A/AAAA address is available
  • select first connect attempt after that with HTTPS-RR info there or not.
  • select second connect attempt only when HTTPS-RR is resolved (may have resolved to "not known") and select possible ALPN from things known by then. May not select any second attempt when first already covers everything.

This means when the HTTPS-RR is known at/before the first address is resolved, everything behaves as before. When the HTTPS-RR is late, a first connection attempt will have been started. Any ALPN preference from the HTTPS-RR that is not already ongoing will then start the second attempt.

For HTTPS-RRs that recommend 2 or more ALPNs, the first will always be attempted: either it is already ongong or it will be the ALPN for the second attempt. The 2nd ALPN recommendation from HTTPS-RR may be honored or not, depending on what is already selected.

The difference in behaviour between early/late HTTPS-RR resolve cannot be helped - unless we do not perform any attempts before it arrives. Trade offs.

Make cf-https-connect work async correctly:
- only start first baller when at least one A/AAAA address
  is available
- select first connect attempt after that with HTTPS-RR info
  there or not.
- select second connect attempt only when HTTPS-RR is resolved
  (may have resolved to "not known") and select possible ALPN
  from things known by then. May not select any second attempt
  when first already covers everything.

This means when the HTTPS-RR is known at/before the first address
is resolved, everything behaves as before. When the HTTPS-RR is
late, a first connection attempt will have been started. Any
ALPN preference from the HTTPS-RR that is not already ongoing will
then start the second attempt.

For HTTPS-RRs that recommend 2 or more ALPNs, the first will always
be attempted: either it is already ongong or it will be the ALPN
for the second attempt. The 2nd ALPN recommendation from HTTPS-RR
*may* be honored or not, depending on what is already selected.

The difference in behaviour between early/late HTTPS-RR resolve
cannot be helped - unless we do not perform any attempts before
it arrives. Trade offs.
@icing icing requested a review from bagder April 8, 2026 10:31
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the HTTPS-CONNECT “eyeballing” logic to behave correctly when HTTPS-RR information arrives asynchronously, so initial connection attempts can start as soon as an A/AAAA is available while still honoring later HTTPS-RR ALPN guidance when it becomes known.

Changes:

  • Reworked cf-https-connect state machine/baller selection to defer choosing the second attempt until HTTPS-RR is resolved.
  • Added a DNS helper (Curl_conn_dns_has_any_ai) to detect when at least one address is available before starting connection attempts.
  • Improved the HTTP/3-over-UNIX-socket failure path to emit a proper error message.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File Description
lib/cf-https-connect.c Major refactor of async “eyeballing”/ALPN attempt selection and connection state handling
lib/cf-dns.c Adds Curl_conn_dns_has_any_ai() helper for partial async DNS availability
lib/cf-dns.h Declares the new DNS helper API
lib/vquic/vquic.c Emits a failf() error message for HTTP/3 over UNIX domain sockets

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lib/cf-https-connect.c
Comment thread lib/cf-https-connect.c
Comment thread lib/cf-https-connect.c Outdated
Comment thread lib/cf-dns.c
@bagder bagder closed this in 567803d Apr 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants