Skip to content

Doh: fix and improvements - #22514

Closed
icing wants to merge 4 commits into
curl:masterfrom
icing:dnsd-doh-fixes
Closed

icing wants to merge 4 commits into
curl:masterfrom
icing:dnsd-doh-fixes

Conversation

@icing

@icing icing commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

DoH improvements

  • decode results when individual requests are done
  • makes happy eyeballing start asap
  • remove doh_resp structures as no longer needed
  • remove CURL_DNS_TYPE_NS, CURL_DNS_TYPE_CNAME and CURL_DNS_TYPE_DNAME from DoH
  • DoH: do not set PIPEWAIT and SSL OPTS when url starts with http:
  • mark Doh master handle as dirty after every sub-request, not only the last
  • Doh: start probe on AAAA before A (was the other way).

cf-dns: set EXPIRE_HAPPY_EYEBALLS timer when waiting 50ms on AAAA result or progress will not be triggered in time.

Add debug env var CURL_DBG_HE_AAAA_AWAIT_MS to override the default 50ms on waiting for the AAAA result to arrive.

test2100: set CURL_DBG_HE_AAAA_AWAIT_MS to 60 seconds to provide enough time for slow CI runs to sent all DoH requests.

@github-actions github-actions Bot added the tests label Aug 7, 2026
@icing icing changed the title Doh: fix error handling Doh: fix and improvements Aug 7, 2026
@icing

icing commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

Ok, the single remaining failure is a Valgrind CI job test2100, that does DoH with A+AAAA queries, but since DoH now supports eyeballing intermediate results, the A answer is acted upon without even enough time to properly start the AAAA query (50ms is the "delay").

That's my current analysis. Not sure how we'd want to stabilize that. Maybe a new env var, like CURL_DEBUG_H3_EYEBALLING_AAAA_WAIT_MS_WHATEVER_ITS_THE_WEEKEND, or so.

@icing icing mentioned this pull request Aug 8, 2026
@icing
icing marked this pull request as ready for review August 8, 2026 11:17
@icing
icing requested a review from bagder August 8, 2026 11:45
@bagder
bagder requested a balanced review from Copilot August 8, 2026 11:56

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Improves DoH response handling and Happy Eyeballs timing while simplifying DoH data structures.

Changes:

  • Decodes each DoH response as it completes.
  • Adds configurable AAAA waiting and timer handling.
  • Expands DoH integration tests and server readiness checks.

Reviewed changes

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

Show a summary per file
File Description
lib/vdns/doh.c Reworks DoH probes, decoding, and result handling.
lib/vdns/doh.h Simplifies DoH types and response state.
lib/vdns/cf-dns.c Adds AAAA waiting configuration and timer.
docs/libcurl/libcurl-env-dbg.md Documents the debug timing variable.
tests/unit/unit1650.c Updates DNS decoder expectations.
tests/http/testenv/dnsd.py Adds HTTP-based server readiness checks.
tests/http/test_21_resolve.py Extends partial-response and HTTP-failure coverage.
tests/data/test2100 Increases AAAA waiting for slow test runs.
Suppressed comments (1)

lib/vdns/doh.c:1122

  • Errors from HTTPS RR decoding, including CURLE_OUT_OF_MEMORY, are likewise reclassified as an HTTP failure and ultimately returned as a name-resolution failure. Preserve this CURLcode in the async result and propagate it from Curl_doh_take_result() instead of losing the actual local error.
    if(result) {
      dohp->probe_rc[slot] = DOH_HTTP_FAILED;
      infof(doh, "[DoH] error decoding HTTPS RR: %s",
            curl_easy_strerror(result));
      goto out;

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread lib/vdns/doh.c
Comment thread lib/vdns/cf-dns.c
Comment thread lib/vdns/doh.c Outdated
Comment thread lib/vdns/doh.c Outdated
Comment thread lib/vdns/doh.c Outdated
Comment thread lib/vdns/doh.c
Comment thread tests/http/testenv/dnsd.py Outdated
icing added 3 commits August 10, 2026 09:20
- decode results when individual requests are done
- makes happy eyeballing start asap
- remove doh_resp structures as no longer needed
- remove CURL_DNS_TYPE_NS, CURL_DNS_TYPE_CNAME and
  CURL_DNS_TYPE_DNAME from DoH
- DoH: do not set PIPEWAIT and SSL OPTS when url starts with http:
- mark Doh master handle as dirty after every sub-request, not only the last
- Doh: start probe on AAAA before A (was the other way).

cf-dns: set EXPIRE_HAPPY_EYEBALLS timer when waiting 50ms on AAAA
result or progress will not be triggered in time.

Add debug env var CURL_DBG_HE_AAAA_AWAIT_MS to override the
default 50ms on waiting for the AAAA result to arrive.

test2100: set CURL_DBG_HE_AAAA_AWAIT_MS to 60 seconds to
provide enough time for slow CI runs to sent all DoH requests.

commit 20bac7a
Author: Stefan Eissing <stefan@eissing.org>
Date:   Fri Aug 7 09:42:51 2026 +0200

    Doh: fix error handling

    Fix DoH error handling when connections failed or invalid HTTP
    status codes were returned. Trace those errors at infof level.

    Add test for 404 response handling.
- fix await_ms comment
- set answered query bit
- Doh: inherit DEBUG settings independent of SSL
- Doh: treat addrinfo conversion error as hard failure,
       preserve and forward CURLcode
- Doh: treat CNAME/DANE sub-responses not as failure
- Doh: treat empty address list not as failure
- dnsd: close logs after stopping server
@icing
icing requested a review from bagder August 10, 2026 10:06
@bagder bagder closed this in 2d30fd2 Aug 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

3 participants