← 返回首页
fix: add pre-read guard to limitedReader to prevent state contamination by Kunall7890 · Pull Request #3033 · labstack/echo · GitHub
Skip to content

Navigation Menu

Toggle navigation
Sign in
Appearance settings
Search or jump to...

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Include my email address so I can be contacted

Saved searches

Use saved searches to filter your results more quickly

Appearance settings
Resetting focus

fix: add pre-read guard to limitedReader to prevent state contamination#3033

Open
Kunall7890 wants to merge 1 commit into
labstack:masterfrom
Kunall7890:fix-body-limit-contamination
Open

fix: add pre-read guard to limitedReader to prevent state contamination#3033
Kunall7890 wants to merge 1 commit into
labstack:masterfrom
Kunall7890:fix-body-limit-contamination

Conversation

Copy link
Copy Markdown

What

Adds a strict pre-read guard to \limitedReader.Read\ that checks whether the read counter (
.read) already exceeds \LimitBytes\ before delegating to the underlying reader.

Why

The \limitedReader\ is reused via \sync.Pool\ in \BodyLimitWithConfig. While \Reset\ clears the counter, a contaminated \limitedReader\ retrieved from the pool could delegate reads to the underlying body without proper rejection if:

  1. \Reset\ is not called before \Read\ (defensive against future refactoring)
  2. A partial read sequence leaves
    .read\ at an intermediate value that still exceeds the limit

The post-read guard alone is insufficient because it only catches the overflow after it happens. The pre-read guard provides defense-in-depth: if
.read\ is already beyond the limit, fail immediately without touching the underlying stream.

Changes

\\diff
-func (r *limitedReader) Read(b []byte) (n int, err error) {

  • n, err = r.reader.Read(b)
    +func (r *limitedReader) Read(p []byte) (n int, err error) {
  • if r.read > r.LimitBytes {
  • return 0, echo.ErrStatusRequestEntityTooLarge
  • }
  • n, err = r.reader.Read(p)
    r.read += int64(n)
    if r.read > r.LimitBytes {
    return n, echo.ErrStatusRequestEntityTooLarge
    }
    return
    }
    \\

Closes #1

When limitedReader is reused via sync.Pool, the read counter (r.read) may retain a stale value if Reset is not called or is called after a partial read. Add a strict pre-read guard that rejects immediately if r.read already exceeds LimitBytes, preventing delegation to the underlying reader with a contaminated counter. Closes labstack#1

aldas commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

so this is same as #3032 but for v5 and missing tests. Please port that PR properly over to v5 with tests

aldas 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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

previous comment

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Sync pool resetting context

2 participants

Footer

© 2026 GitHub, Inc.