Skip to content

Apply LimitedReader structs to all io.ReadAll calls.#8834

Open
ezekiel wants to merge 4 commits into
mainfrom
ezekiel/use-limited-reader
Open

Apply LimitedReader structs to all io.ReadAll calls.#8834
ezekiel wants to merge 4 commits into
mainfrom
ezekiel/use-limited-reader

Conversation

@ezekiel

@ezekiel ezekiel commented Jul 1, 2026

Copy link
Copy Markdown
Member

Fixes #8823

@ezekiel ezekiel requested a review from a team as a code owner July 1, 2026 16:21
@ezekiel ezekiel requested a review from aarongable July 1, 2026 16:21

@aarongable aarongable 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.

High-level comments:

  1. Let's not bother in all the test files (definitely foo_test.go, and maybe test/blah at your discretion)
  2. Let's crank down the limit for most of these (100MB is too many)
  3. Let's crank up the limit specifically for anything reading CRLs, since those could exceed 100MB in a mass revocation event
  4. Maybe a core.ReadAtMost(reader, limit) helper with accompanying core.DefaultMaxRead const? Also up to you.

ezekiel added 2 commits July 2, 2026 18:14
Reduce to still very generous ~300K normal reader size.
Increase to ~1G CRL reader size.
@ezekiel

ezekiel commented Jul 2, 2026

Copy link
Copy Markdown
Member Author

This reduced set of changes gets us good coverage for production code that we can deploy with lower impact risks. And we still have the option to put up a second iteration with a core helper.

@ezekiel ezekiel requested a review from aarongable July 2, 2026 22:11
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.

Use bounded readers where we currently use io.ReadAll

2 participants