Skip to content

fix: implement io.ReaderFrom in wrapped response#17

Merged
samber merged 1 commit intosamber:mainfrom
networkteam:main
Feb 2, 2026
Merged

fix: implement io.ReaderFrom in wrapped response#17
samber merged 1 commit intosamber:mainfrom
networkteam:main

Conversation

@hlubek
Copy link
Contributor

@hlubek hlubek commented Feb 2, 2026

For compatibility with other middlewares (e.g. chi) that would otherwise skip a Hijacker interface.

@samber
Copy link
Owner

samber commented Feb 2, 2026

LGTM 👍

@samber samber merged commit fa15b80 into samber:main Feb 2, 2026
6 checks passed
return n, err
}
}
return io.Copy(w, r)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@samber this line creates a stackoverflow on the new release from yesterday. Since w implements io.ReaderFrom now, io.Copy will call ReadFrom to perform the copying. This creates a stackoverflow on the first write.

The fix would be to call io.Copy(w.ResponseWriter, r) (or similar) instead.

I can create an issue for this if needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I missed testing the case with body != nil.

Using io.Copy(w.ResponseWriter, r) will not write to the body, so we need to find another way to not use ReadFrom in io.Copy.

I can have a look at it and also add a test to make sure both cases work.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a fix and tests in #18

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.

3 participants