Fix custom HTTP response status codes arriving as 200 at the client#12
Open
sclaiborne wants to merge 3 commits into
Open
Fix custom HTTP response status codes arriving as 200 at the client#12sclaiborne wants to merge 3 commits into
sclaiborne wants to merge 3 commits into
Conversation
Covers the LLHttpResponse status input end-to-end through the server send path, plus LLHttpBuildResponse unit coverage for non-200 codes, the status 0 default, and content-length on empty payloads. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
LLHttpUriMatch returned a match as soon as the request URI reached its query string, without requiring the handler pattern to be consumed too. A request like 'POST /api/brew?durationS=180' therefore dispatched to a handler listening on '/api/brew/abort' as well as the '/api/brew' one. Every matching LLHttpResponse FB then answered the same request, and whichever response was queued first reached the client - typically a sibling route's 200 that masked the real route's custom status (202, 409, ...). Requests without a query string were unaffected, which made the bug look like the status input was ignored. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A response queued with status 0 (the FB input's initial value) went out as the malformed status line 'HTTP/1.1 0 ', which strict clients such as .NET HttpClient reject outright. Default to 200 OK so applications that never set the status input keep working. Responses with an empty payload now carry an explicit content-length: 0 (except 1xx/204/304, which must not have one) so keep-alive clients do not wait for a body that never comes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Reported against
@loupeteam/llhttp1.0.0 (ARsim, AR 6.5.1): thestatusinput ofLLHttpResponseappears to be ignored — every response arrives at the client asHTTP/1.1 200 OK, even when the application sets 202/409/400 in the same cycle assend := TRUE.Root cause
LLHttpUriMatch()returned a match as soon as the request URI reached its query string, without requiring the handler pattern to be consumed too (HttpMisc.c):So
POST /api/brew?durationS=180matched a handler listening on/api/brew/abortin addition to the/api/brewone. The server dispatches to every matching handler, so bothLLHttpResponseFBs answered the same request, and whichever response was queued first reached the client — typically a sibling route''s 200 that masked the real route''s 202/409. Requests without a query string dispatched correctly, which is what made the symptom look like "the status input is ignored": the response body came from the right family of routes while the status was always 200.The status input itself was never broken: driving the full
LLHttpServer+LLHttpResponsepath in the off-PLC harness shows 200/202/204/400/404/409/500 all reaching the wire correctly, on this branch and on the v1.0.0 sources. (The published 1.0.0 binary was also ruled out: it was built by CI three minutes after thev1.0.0tag commit, and the packaged.fun/.typare byte-identical to the tag.)Changes
LLHttpUriMatch: a query string in the request URI only counts as a match if the handler pattern is exhausted at that point. Wildcard patterns (*,**) and exact matches with queries behave as before.LLHttpBuildResponse: a response whose status was left at 0 now goes out as200 OKinstead of the malformedHTTP/1.1 0status line (which strict clients such as .NETHttpClientreject outright).LLHttpBuildResponse(in passing): empty-payload responses now carry an explicitcontent-length: 0(except 1xx/204/304, which must not have one), so keep-alive clients don''t wait for a body that never comes.Tests
LLHttpServer+LLHttpResponsedriven through the stubbed TCP layer; asserts the wire status line for 200, 202, 204, 400, 404, 409, 500, and the status-0 default.POST /api/brew,POST /api/brew/abort), a query-string request, and an immediate 200 from the sibling route — asserts the sibling never sees the request and the wire carries the 202.LLHttpUriMatchunit cases for query-string URIs (including the/api/brew?xvs/api/brew/abortfalse match).LLHttpBuildResponseunit cases for non-200 status, status-0 default,content-length: 0, and 204 omitting content-length.All 11 test cases pass off-PLC (32-bit MinGW via the loupeRT harness); the 4 new/extended ones fail without the fix.
Notes
HttpMisc.cand the response-build path inHttpServer.c, so there is no overlap with the receive-path fixes. When Guard server receive window against rollover and bound debug client copy #11 merges this PR should retarget tomainautomatically.🤖 Generated with Claude Code