Skip to content

digest: parse without storing 'value' in local buffer - #22831

Closed
bagder wants to merge 3 commits into
masterfrom
bagder/digest-parser
Closed

bagder wants to merge 3 commits into
masterfrom
bagder/digest-parser

Conversation

@bagder

@bagder bagder commented Sep 4, 2026

Copy link
Copy Markdown
Member

Use of curlx_str_ functions avoids having a copy.

Use of curlx_str_ functions avoids having a copy.
@bagder
bagder marked this pull request as ready for review September 4, 2026 12:36
@bagder
bagder requested a lite review from Copilot September 4, 2026 12:36

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.

🟡 Changes recommended

lib/vauth/digest.h now references struct Curl_str without a forward declaration or include, which breaks compilation for translation units including this header.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR updates Digest authentication challenge parsing to avoid copying directive names into a local buffer by switching to curlx_str_* parsing helpers and struct Curl_str views.

Changes:

  • Change Curl_auth_digest_get_pair() to return the directive name via struct Curl_str instead of a fixed-size char[] buffer.
  • Replace manual blank-skipping and string comparisons with curlx_str_passblanks() and curlx_str_casecompare().
  • Add curlx/strparse.h include where needed for the new parsing helpers.
File summaries
File Description
lib/vauth/digest.h Updates Curl_auth_digest_get_pair() signature to use struct Curl_str *.
lib/vauth/digest.c Refactors digest challenge parsing to use curlx_str_* helpers and Curl_str comparisons.
lib/vauth/digest_sspi.c Refactors SSPI digest parsing similarly and adds curlx/strparse.h.
Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 1
  • Review effort level: Lite

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

Comment thread lib/vauth/digest.h
@testclutch

Copy link
Copy Markdown

Analysis of PR #22831 at 9fcfa933:

Test ../../tests/http/test_19_shutdown.py::TestShutdown::test_19_03_shutdown_by_server[http/1.1] failed, which has NOT been flaky recently, so there could be a real issue in this PR. Note that this test has failed in 2 different CI jobs (the link just goes to one of them). Note that this CI job has had a number of other flaky tests recently (2, to be specific) so it may be that this failure is rather a systemic issue with this job and not with this specific PR.

Generated by Testclutch

@bagder bagder closed this in 9be5c68 Sep 4, 2026
@bagder
bagder deleted the bagder/digest-parser branch September 4, 2026 15:43
bagder pushed a commit that referenced this pull request Sep 5, 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