Skip to content

Conversation

@bagder
Copy link
Member

@bagder bagder commented Dec 27, 2025

Also renamed the struct field to 'h1hdr' from 'scratch' to better say what its purpose is.

Also renamed the struct field to 'h1hdr' from 'scratch' to better say
what its purpose is.
@bagder bagder added the HTTP/3 h3 or quic related label Dec 27, 2025
@bagder bagder marked this pull request as ready for review December 27, 2025 09:29
@bagder bagder requested a review from Copilot December 27, 2025 09:29
Copy link

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 adds security validation to refuse HTTP/3 headers containing CR, LF, or null bytes, and improves code clarity by renaming the scratch field to h1hdr to better reflect its purpose as a buffer for HTTP/1-style header construction.

Key Changes:

  • Introduced fineh3header() validation function to check header values for invalid characters (CR, LF, null)
  • Renamed struct field scratch to h1hdr throughout the codebase for better clarity
  • Added validation check before processing regular headers

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

@bagder
Copy link
Member Author

bagder commented Dec 27, 2025

augment review

@augmentcode
Copy link

augmentcode bot commented Dec 27, 2025

🤖 Augment PR Summary

Summary: Hardens curl’s quiche-based HTTP/3 response header handling by rejecting header fields containing CR/LF/NUL, preventing unsafe HTTP/1-style header reconstruction.

Changes:

  • Rename the temporary header construction buffer from scratch to h1hdr for clarity
  • Add is_valid_h3_header() to detect CR, LF, and NUL bytes in header names/values
  • Validate :status before decoding and synthesizing the HTTP/3 ...\r\n status line
  • Only emit HTTP/1-formatted header lines for name/value pairs that pass validation; otherwise log and ignore

Technical Notes: This change reduces the risk of header injection or parser confusion when converting HTTP/3 headers into HTTP/1-like lines for downstream processing.

🤖 Was this summary useful? React with 👍 or 👎

Copy link

@augmentcode augmentcode bot left a comment

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.

@bagder bagder closed this in 6842d4e Dec 27, 2025
@bagder bagder deleted the bagder/quiche-bad-headers branch December 27, 2025 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

HTTP/3 h3 or quic related

Development

Successfully merging this pull request may close these issues.

1 participant