Skip to content

fix(server): answer an unreadable form body as invalid_request - #512

Merged
osanderson merged 1 commit into
mainfrom
fix/server-fromhttp-errors
Oct 2, 2026
Merged

osanderson merged 1 commit into
mainfrom
fix/server-fromhttp-errors

Conversation

@osanderson

Copy link
Copy Markdown
Collaborator

Summary

This addresses DevX review item 5. FormRequestFromHTTP, and the three constructors built on it (PushAuthorizationRequestFromHTTP, TokenEndpointRequestFromHTTP, BeginBackchannelAuthenticationRequestFromHTTP), returned plain fmt errors for a body they couldn't take. So the natural server.WriteError(w, err) answered a malformed request with a 500 server_error, contrary to WriteError's doc ("every error this package's own methods return" is a *Error). BeginAuthorizationRequestFromHTTP (#505) already returned a 400 *Error.

Each failure is now a 400 invalid_request *Error. Its public description is fixed and doesn't echo input, and the detail stays in the cause for logs:

Failure Public description
Wrong content type "the request body must be application/x-www-form-urlencoded"
Unreadable "the request body could not be read"
Over 1 MiB "the request body is too large"
Malformed name or value "the request body is malformed"

It isn't breaking: the functions still return error, now always a *server.Error.

Call sites:

  • The six demos: their badRequest helpers (log plus a generic 400) are deleted, and the call sites use server.WriteError.
  • cmd/conformance-as: the PAR, token and CIBA handlers used writeRawOAuthError(..., err.Error()), which sent the internal error text to the client. They now use writeOAuthJSONError (server.WriteError).

Tests

  • TestFromHTTPBodyFailuresAreInvalidRequest: five failures (wrong content type, malformed name, malformed value, oversized, unreadable body) across all four constructors. Each is a 400 invalid_request *server.Error, and WriteError writes 400 "invalid_request".
  • Mutation check: turning one failure back into a plain error fails the test.
  • Other checks:
    • go test ./server/... ./cmd/... passes.
    • golangci-lint is clean.
    • All six demos' vet, tests and lint pass.

🤖 Generated with Claude Code

@codecov

codecov Bot commented Oct 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

FormRequestFromHTTP, and the PushAuthorizationRequestFromHTTP,
TokenEndpointRequestFromHTTP and
BeginBackchannelAuthenticationRequestFromHTTP constructors built on it,
returned plain errors for a body they couldn't take: not form-encoded,
unreadable, over 1 MiB, or malformed. So server.WriteError, whose doc
says every error this package returns is a *Error, answered the
client's mistake with a 500 server_error. Each is now a 400
invalid_request *Error with a fixed public description; the detail stays
in the cause, for logs.

BeginAuthorizationRequestFromHTTP already did this. The demos' badRequest
helpers and the conformance AS's err.Error() echo (which sent the
internal message to the client) give way to server.WriteError.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@osanderson
osanderson force-pushed the fix/server-fromhttp-errors branch from e7cf95e to ee2f80a Compare October 2, 2026 06:11
@sonarqubecloud

sonarqubecloud Bot commented Oct 2, 2026

Copy link
Copy Markdown

@osanderson
osanderson merged commit 3544ad9 into main Oct 2, 2026
17 checks passed
@osanderson
osanderson deleted the fix/server-fromhttp-errors branch October 2, 2026 06:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant