Multipart encoder can panic on typed nil io.Reader pointers
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 697
- Forks
- 60
- Avg merge
- 2d 21h
- Merged PRs (30d)
- 71
Description
Summary
internal/apiform detects io.Reader implementations before it applies the encoder's existing nil-pointer handling. A typed nil pointer whose type implements io.Reader can therefore be routed into io.Copy, which invokes Read on a nil receiver and can panic.
The encoder already defines nil pointers as empty form fields, so reader detection currently bypasses an intended invariant.
Reproduction
On current main (d082a010f7c6cacf407d8a1581446a7857f9f1bb), a minimal reader type demonstrates the problem:
type panicReader struct{}
func (*panicReader) Read([]byte) (int, error) {
panic("Read called on nil receiver")
}
var reader *panicReader
var buf bytes.Buffer
writer := multipart.NewWriter(&buf)
_ = Marshal(map[string]any{"file": reader}, writer)
Because *panicReader implements io.Reader, encodeValue checks:
if t.Implements(reflect.TypeOf((*io.Reader)(nil)).Elem()) {
return e.encodeReader(key, val, writer)
}
before reaching the pointer branch:
case reflect.Pointer:
if val.IsNil() || !val.IsValid() {
return writer.WriteField(key, "")
}
The nil pointer is therefore converted to an io.Reader interface and passed to io.Copy, which calls its Read method.
Expected behavior
A typed nil pointer should follow the same nil-pointer semantics regardless of whether its type happens to implement io.Reader: encode an empty field and do not invoke methods on the nil receiver.
Suggested fix
Check nil pointer/interface values before reader-interface detection. Keep reader handling unchanged for non-nil values.
Add a regression using a typed nil reader whose Read method panics if invoked, and verify multipart encoding completes without a panic and emits an empty field.
Impact
This is robustness and request-serialization correctness. A legitimate typed nil value can currently turn request construction into a process panic instead of being handled by the encoder's existing null-value path.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in internal/apiform at encodeValue and compare its nil-pointer handling with reader detection and encodeReader. Add a regression using the typed nil panicReader from the reproduction, then verify multipart encoding completes without invoking Read and emits an empty field.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- api
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 84/100