-
Notifications
You must be signed in to change notification settings - Fork 18.4k
Description
New Proposal
As per @ianlancetaylor's suggestion, this proposal is now to add an Err field to io.LimitedReader. If not nil, this field's value will be returned when trying to read past the limit imposed by the reader instead of the default io.EOF.
Original Proposal
For various reasons, I had a need to use MaxBytesReader() in a situation where a ResponseWriter wasn't easily available, so I looked through the code to see if it was safe to pass nil and found that all it does is check a type assertion and then call an unexported method on it:
type requestTooLarger interface {
requestTooLarge()
}
if res, ok := l.w.(requestTooLarger); ok {
res.requestTooLarge()
}While it's true that because of this it's safe to pass it a nil ResponseWriter, it feels quite odd to rely on undocumentated behavior of this kind.
Proposal
Several ways to clean this situation up come to mind:
- Add an alternative in the
httppackage that doesn't need aResponseWriter. Because of how it works, it could actually just return the same implementation, but the function will be more obvious in its usage. - Document how the implementation uses the
ResponseWriterand explicitly state that anilResponseWriteris valid. - Add a reimplementation of
MaxBytesReader()iniothat doesn't use aResponseWriterat all and isn't tied tohttpbehavior, but includes the other differences. This could actually be done, for the most part, by just adding anErr errorfield toio.LimitedReaderthat, if non-nil, is the error returned when the limit is reached instead ofio.EOF.
In whichever case, the documentation for MaxBytesReader() should probably be filled in a bit. It is quite odd that something called Reader requires a type that is primarily an io.Writer implementation for non-obvious reasons and doesn't mention it anywhere in the documentation.
Metadata
Metadata
Assignees
Labels
Type
Projects
Status