Skip to content

digest: handle quotes in the path - #20295

Closed
bagder wants to merge 4 commits into
masterfrom
bagder/digest-quotes
Closed

digest: handle quotes in the path#20295
bagder wants to merge 4 commits into
masterfrom
bagder/digest-quotes

Conversation

@bagder

@bagder bagder commented Jan 13, 2026

Copy link
Copy Markdown
Member

The 'uri' component needs to be escaped as well.

Also, use goto as a general error mechanism.

Make test 64 use a double quote in the URL.

@testclutch

This comment was marked as resolved.

@bagder
bagder force-pushed the bagder/digest-quotes branch from f943f97 to 9d0f050 Compare January 14, 2026 07:44
The 'uri' component needs to be escaped as well.

Also, use goto as a general error mechanism.

Make test 64 use a double quote in the URL.

Closes #20295
@bagder
bagder marked this pull request as ready for review January 14, 2026 08:26
@bagder
bagder requested a review from Copilot January 14, 2026 08:26

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

This pull request adds quote and backslash escaping for the URI component in HTTP Digest authentication headers and refactors error handling to use a goto-based cleanup pattern.

Changes:

  • Refactored auth_digest_string_quoted() to use dynamic buffers instead of pre-calculated allocation
  • Updated auth_create_digest_http_message() to use goto-based error handling for cleanup
  • Added URI escaping for the digest authentication Authorization header
  • Modified test64 to include a double quote character in the URL path to verify the escaping functionality

Reviewed changes

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

File Description
tests/data/test64 Updated test case to include a double quote in the URL and updated expected Authorization header and response hash
lib/vauth/digest.c Refactored string quoting function and digest message creation with improved error handling and URI escaping

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

Comment thread lib/vauth/digest.c
Comment thread lib/vauth/digest.c
Comment thread lib/vauth/digest.c Outdated
Comment thread lib/vauth/digest.c Outdated
@bagder
bagder force-pushed the bagder/digest-quotes branch from f239b00 to 4aa5e2b Compare January 14, 2026 08:43
@bagder

bagder commented Jan 14, 2026

Copy link
Copy Markdown
Member Author

augment review

@augmentcode

augmentcode Bot commented Jan 14, 2026

Copy link
Copy Markdown
🤖 Augment PR Summary

Summary: This PR improves HTTP Digest auth header generation when the request path contains quote/backslash characters.

Changes:

  • Reworks quoted-string escaping in lib/vauth/digest.c to use dynbuf rather than manual size pre-calculation.
  • Escapes the Digest uri= parameter so embedded " or \\ in the path are represented correctly inside the quoted-string.
  • Refactors auth_create_digest_http_message() error handling to use a single cleanup label and builds the header value via curlx_dyn_addf().
  • Updates test 64 to include a double quote in the URL and adjusts the expected request line, Digest uri field, and response hash accordingly.

Technical Notes: The Digest uri value is now escaped specifically for header quoting, while the digest hash still uses the raw request URI bytes (as sent in the request line).

🤖 Was this summary useful? React with 👍 or 👎

@augmentcode augmentcode Bot 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.

Review completed. 1 suggestions posted.

Fix All in Augment

Comment augment review to trigger a new review at any time.

Comment thread lib/vauth/digest.c
@bagder bagder closed this in 134fb66 Jan 14, 2026
@bagder
bagder deleted the bagder/digest-quotes branch January 14, 2026 09:10
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