Skip to content

Document HTTPHdr methods for #13420 review - #13422

Merged
JosiahWI merged 7 commits into
apache:masterfrom
JosiahWI:doc/parse-req
Jul 23, 2026
Merged

Document HTTPHdr methods for #13420 review#13422
JosiahWI merged 7 commits into
apache:masterfrom
JosiahWI:doc/parse-req

Conversation

@JosiahWI

@JosiahWI JosiahWI commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

This adds doxygen comments for HTTPHdr::length_get and HTTPdr::parse_req.

This adds doxygen comments for `HTTPHdr::length_get` and `HttpHDR::parse_req`.
@JosiahWI JosiahWI self-assigned this Jul 23, 2026
Copilot AI review requested due to automatic review settings July 23, 2026 13:52
@JosiahWI JosiahWI changed the title Document HTTP methods for #13420 review Document HTTPHdr methods for #13420 review Jul 23, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds/expands Doxygen documentation for HTTPHdr header serialization sizing and incremental HTTP/1.x request parsing APIs, improving the discoverability of these core header utilities in ATS’s proxy header layer.

Changes:

  • Documented HTTPHdr::length_get() to clarify what the serialized length includes/excludes.
  • Added detailed Doxygen comments for both HTTPHdr::parse_req() overloads (raw buffer + IOBufferReader), including incremental parsing behavior, limits, and return values.
Comments suppressed due to low confidence (1)

include/proxy/hdrs/HTTP.h:674

  • Same documentation consistency issue as the raw-buffer overload: prefer @a <name> for parameter references in prose (instead of @p) and use @param[in] / @param[out] / @param[in,out] so direction is part of the Doxygen tag rather than embedded in the description. This matches other doxygenized headers in include/proxy/hdrs/.
   * Reads and consumes data from @p r, parsing it into the header's request fields. Call
   * repeatedly with the same @p parser until a result other than @c ParseResult::CONT is
   * returned. When @c ParseResult::DONE is returned, the request method, URL, version, and
   * header fields are set on this header.
   *

Comment thread include/proxy/hdrs/HTTP.h
Copilot AI review requested due to automatic review settings July 23, 2026 14:32

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comment thread include/proxy/hdrs/HTTP.h Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 23, 2026 14:36

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Comment thread include/proxy/hdrs/HTTP.h Outdated
Comment thread include/proxy/hdrs/HTTP.h Outdated
Copilot AI review requested due to automatic review settings July 23, 2026 16:07
  Put brief sentence on opening line
  Use in/out/in,out parameter markers

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Comment thread include/proxy/hdrs/HTTP.h Outdated
Copilot AI review requested due to automatic review settings July 23, 2026 16:30

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

@JosiahWI
JosiahWI requested a review from bneradt July 23, 2026 16:42

@bneradt bneradt left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good. A couple other thoughts.

Comment thread include/proxy/hdrs/HTTP.h Outdated
Comment thread include/proxy/hdrs/HTTP.h Outdated
Comment thread include/proxy/hdrs/HTTP.h Outdated
Comment thread include/proxy/hdrs/HTTP.h
  Clarify that `@` headers are also included in length
@JosiahWI
JosiahWI requested review from bneradt and Copilot July 23, 2026 17:05

@bneradt bneradt left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks great. Thanks for working on this.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

include/proxy/hdrs/HTTP.h:689

  • Same as the raw-buffer overload: max_hdr_field_size is enforced as field-name.size() + field-value.size() (excluding the : separator and CRLF), not the full serialized field line length. Updating the wording will make the limit semantics accurate.
   * @param[in]     max_request_line_size Maximum byte length of the request line; exceeding it returns
   *                                       @c ParseResult::ERROR.
   * @param[in]     max_hdr_field_size    Maximum byte length of a single header field; exceeding it
   *                                       returns @c ParseResult::ERROR.

Comment thread include/proxy/hdrs/HTTP.h
@JosiahWI
JosiahWI merged commit e241265 into apache:master Jul 23, 2026
15 checks passed
cmcfarlen pushed a commit to cmcfarlen/trafficserver that referenced this pull request Jul 29, 2026
* Document HTTP methods for apache#13420 review

* Make changes requested by Brian Neradt

  Put brief sentence on opening line
  Use in/out/in,out parameter markers
  Clarify that `@` headers are also included in length

(cherry picked from commit e241265)
@cmcfarlen cmcfarlen moved this to Picked v10.2.0 in ATS v10.2.x Jul 29, 2026
@cmcfarlen cmcfarlen added this to the 10.2.0 milestone Jul 29, 2026
@github-project-automation github-project-automation Bot moved this from Picked v10.2.0 to For v10.2.0 in ATS v10.2.x Jul 29, 2026
@cmcfarlen cmcfarlen moved this from For v10.2.0 to Picked v10.2.0 in ATS v10.2.x Jul 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Picked v10.2.0

Development

Successfully merging this pull request may close these issues.

4 participants