Skip to content

refactor: rename readBody to readErrorBody and cap body size - #1246

Merged
rdimitrov merged 1 commit into
mainfrom
refactor/read-error-body-cap
May 4, 2026
Merged

rdimitrov merged 1 commit into
mainfrom
refactor/read-error-body-cap

Conversation

@rdimitrov

Copy link
Copy Markdown
Member

Summary

Follow-up to #1241. Two small tweaks to the readBody helper:

  • Rename readBody → readErrorBody. The helper is only ever called on the error path (after a non-2xx status check), where we want a small string for diagnostics. The generic name made it look reusable on the success path, where streaming JSON decoding is the right approach. The new name pins the intended use down.
  • Cap the read with io.LimitReader (8 KiB). Defense in depth — error diagnostics never need more than a few KB, and we shouldn't buffer an arbitrarily large body from an upstream just to format an error message. 8 KiB is well above any realistic GitHub API or JWKS error response.

No behavior change on the happy path. On the unhappy path, error messages are now bounded.

Test plan

  • go build ./...
  • go test ./internal/api/handlers/v0/auth/...

🤖 Generated with Claude Code

The helper introduced in #1241 is only called on the error path of
upstream HTTP responses, so the more specific name makes its intent
unambiguous at the call site and prevents accidental reuse on the
success path (where streaming JSON decoding is preferred).

Wrap the read with io.LimitReader so a misbehaving upstream returning
a huge body cannot cause us to buffer it all in memory just to format
an error message. 8 KiB is well above any realistic GitHub or JWKS
error response.
@rdimitrov
rdimitrov enabled auto-merge (squash) May 4, 2026 11:09
@rdimitrov
rdimitrov disabled auto-merge May 4, 2026 11:09
@rdimitrov
rdimitrov merged commit 7a4216c into main May 4, 2026
5 checks passed
@rdimitrov
rdimitrov deleted the refactor/read-error-body-cap branch May 4, 2026 11:10
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