Add RFC 10008 HTTP QUERY method

[dboehmer]

https://datatracker.ietf.org/doc/rfc10008/ introduces the HTTP QUERY method.

I have no prior experience with HTTP-Message development but I tried to mimic docs and code from POST/PUT and OPTIONS which was also added later.

If this is going to merged into a version higher than 7.03 the version number stated in the POD needs to be adjusted.

I plan to add QUERY support to the Catalyst framework but need this as a dependency.

[oalders]

Thanks for this, and for mimicking the existing OPTIONS work — that was the right model to follow. The shape is good and the tests pass. A few things to sort out before merge.

1. QUERY should go in @EXPORT_OK, not @EXPORT

This is the main one. Adding a new symbol to the default export list is a backward-compatibility hazard: any downstream code that does use HTTP::Request::Common; and also defines or imports its own QUERY starts getting a redefinition conflict purely from upgrading. QUERY is a plausible sub name in DB- and search-adjacent code, so the collision risk is real.

DELETE is kept out of the default exports for exactly this reason (see @EXPORT_OK and the "This function is not exported by default." note in its POD). OPTIONS did go into @EXPORT back in 6.21, but I'd rather not repeat that for a name as common as QUERY.

So:

our @EXPORT    = qw(GET HEAD OPTIONS PATCH POST PUT);
our @EXPORT_OK = qw($DYNAMIC_FILE_UPLOAD DELETE QUERY);

Two knock-on changes:

• The QUERY POD needs the same "This function is not exported by default." sentence that DELETE has.
• t/common-req.t does a bare use HTTP::Request::Common; on line 8, so the new test on line 75 will fail to compile once QUERY is no longer a default export. Please add an explicit import (use HTTP::Request::Common qw(QUERY); alongside the existing bare use).

2. Docs

"below" should be "above". The new block says:

The same as POST below, but the method in the request is QUERY

but the QUERY items were inserted at the very end of the =over, after the whole POST description — so "below" points at nothing. PATCH, PUT and OPTIONS all say "below" and are all genuinely above POST.

Cleanest fix is to move the entire QUERY =item block up so it sits immediately after the OPTIONS block and before =item POST $url. That keeps the "same as POST below" family together and makes the sentence true as written, rather than editing the wording.

SYNOPSIS. It lists POST, PATCH, PUT and OPTIONS but not QUERY — worth adding a line there.

Version. 7.03 is correct as things stand — $VERSION is already '7.03', so the "added in version 7.03" note and the use HTTP::Request::Common 7.03; example need no change.

Changes. The convention in this file includes the issue reference, e.g. - now handling HTTP method '0' (GH#211) (Karen Etheridge). Please add (GH#225) to the new entry.

3. Tests

Nothing asserts the method is actually QUERY. The two new tests check content and Content-Length, neither of which would fail if sub QUERY dispatched to 'PATCH' by mistake — which is the one thing this PR exists to guarantee. PUT and OPTIONS both get an is($r->method, ...); QUERY should too.

Please mirror the OPTIONS block rather than the PATCH one-liner. OPTIONS gets a full block covering the scalar Content => 'foo' path, the method, the host, the absence of a leaked Content pseudo-header, content_ref, content_length, and the $form_ref case. QUERY currently gets only the $form_ref one-liner. Copying the OPTIONS block is about eight lines and actually exercises the code path.

application/octet-steam is a typo for octet-stream. It's pre-existing in the PUT and PATCH tests you copied from and it's inert here, but let's not propagate it — worth fixing in the new test at minimum.

The # a slight lie comment was copied along with the rest of that block. Where a test needs explaining, this project would rather see it in the test description than in a comment — so something like is($r->header('Content-Length'), 9, 'Content-Length is recalculated, not taken from the header'); and drop the comment.

4. RFC 10008 conformance — checked, and you're fine

I went through the RFC rather than assume routing the method through request_type_with_data was sufficient. It is, but for the record:

• Content-Type is mandatory — the RFC says servers MUST fail a QUERY whose Content-Type is missing. request_type_with_data always sets one, defaulting to application/x-www-form-urlencoded, so we can never emit a non-conformant request. Even a bare QUERY $url produces a Content-Type alongside Content-Length: 0.
• Safe, idempotent, cacheable — nothing a request builder needs to enforce.
• The application/x-www-form-urlencoded default is inherited from POST and is a touch arbitrary for QUERY, given the RFC anticipates purpose-built query formats advertised via Accept-Query. It's overridable and consistent with the rest of the module, so I'm happy to leave it — just noting it's a default rather than a considered choice.

Two things that are genuinely out of scope for this PR, but which you'll want to know about given the Catalyst plan:

• Redirects. RFC 10008 is explicit that the POST exception does not carry over: a QUERY receiving a 301 or 302 must be repeated as a QUERY, not downgraded to GET (303 still becomes a GET). That logic lives in LWP::UserAgent, not here, so it needs a companion PR against libwww-perl. Without it, a redirected QUERY will silently turn into a GET and drop its content — probably the most likely real-world breakage.
• Accept-Query. HTTP::Headers doesn't have it in its known-header tables. The practical effect is small: unknown fields already get title-cased automatically, so it renders as Accept-Query correctly, and only its position in the sorted as_string output differs. Adding it to @response_headers would be a reasonable small follow-up, but it isn't this PR's job.

Everything else looks good

• The expanded method list in HTTP::Request's method POD is accurate and nicely alpha-sorted.
• Alpha-sorting @EXPORT and the sub definitions, plus the =qw → = qw spacing fix, are welcome tidy-ups.
• The C<Croak> → C<croak> fix is unrelated but fine, and I appreciate it being in its own commit.

[oalders]

One addition to the docs section of my review, which I should have caught the first time.

RFC 10008 is referenced in the PR title, the PR description and the Changes entry, but nowhere in the POD. Someone running perldoc HTTP::Request::Common sees a QUERY function described only as "the same as POST" and "added in version 7.03" — with nothing to say what QUERY actually is or where it comes from.

Citing the governing RFC inline is the convention in this distribution: HTTP::Status annotates nearly every non-baseline status code with its RFC, and HTTP::Headers cites them in both comments and POD prose. The OPTIONS block you modelled this on doesn't cite one, but that's because OPTIONS has been in the base HTTP spec since forever and there's nothing interesting to point at. QUERY was published last month and most readers won't have encountered it yet.

Since I've already asked you to touch this line for the "below" → "above" fix, it's one edit rather than two:

The same as C<POST> above, but the method in the request is C<QUERY>, the
safe, idempotent method with content defined by
L<RFC 10008|https://www.rfc-editor.org/rfc/rfc10008.html>.

Word it however you prefer — the point is just that the POD should give a reader some idea what QUERY is and a link to follow, which the current text doesn't.

[oalders]

Filed the redirect follow-up: libwww-perl/libwww-perl#530.

Correcting what I said in the review — only 302 is affected. LWP::UserAgent already leaves 301, 307 and 308 alone, and 303→GET is correct per the RFC. So the only gap is a QUERY hitting a 302, which gets downgraded to GET with its content dropped.

Out of scope for this PR either way. Nothing to change here.

[dboehmer]

I updated the PR. Only 2 small objections:

I think these two statements contradict:

Cleanest fix is to move the entire QUERY =item block up so it sits immediately after the OPTIONS block and before =item POST $url. That keeps the "same as POST below" family together and makes the sentence true as written, rather than editing the wording.

The expanded method list in HTTP::Request's method POD is accurate and nicely alpha-sorted.

I left the list item at the bottom and changed the text.

Version. 7.03 is correct as things stand — $VERSION is already '7.03', so the "added in version 7.03" note and the use HTTP::Request::Common 7.03; example need no change.

There was a 7.03 release in the meantime. So I changed to 7.04.

Thanks for your extensive and useful feedback! 👍 It reads like you used AI to analyze the PR and write comments but edited/extended it manually, didn’t you? I feel like this is a good way to utilize AI but still give me human interaction. Eventually it would have been a bit easier to work on your comments if you had commented on lines of code in the diff view instead of posting a large chunk of text.

[oalders]

Thanks for your extensive and useful feedback! 👍 It reads like you used AI to analyze the PR and write comments but edited/extended it manually, didn’t you?

I did. Normally I like to have the agent sign it with name and model, but that seems not to happen consistently and it didn't happen here. I'll have to see about enforcing that so it doesn't look like I'm trying to pass off LLM text as my own.

Eventually it would have been a bit easier to work on your comments if you had commented on lines of code in the diff view instead of posting a large chunk of text.

💯 I thought this wasn't possible using command line tools, but I may actually be wrong. I will try to improve how the reviews are created. Thanks for the quick turnaround. I actually didn't see any kind of notification about this pull request, which is why it sat without comment. Not sure what's going on there.

[oalders]

👍

🤖 Posted by Claude Code (model: Claude Opus 4.8, claude-opus-4-8) on behalf of @oalders

Dismissing: all requested changes have been addressed in the latest commits (QUERY moved to @EXPORT_OK, POD/SYNOPSIS/Changes updates, method assertion + expanded tests, typo fix).

🤖 Dismissed by Claude Code (model: Claude Opus 4.8, claude-opus-4-8) on behalf of @oalders

[oalders]

Approving — all the earlier requested changes are addressed:

• QUERY moved to @EXPORT_OK (opt-in), with the "not exported by default" POD note.
• POD "below" → "above" fixed, SYNOPSIS updated, Changes now credits (GH#225).
• Tests now assert method is QUERY, mirror the fuller OPTIONS block (scalar content, host, no leaked Content header, content_ref, content_length), and fix the octet-steam typo; the # a slight lie comment is folded into the test description.

🤖 Approved by Claude Code (Opus 4.8) on behalf of @oalders