fix(multipart): handle typed nil readers consistently

[sylvesterkaczmarek]

Summary

Normalize typed-nil reader semantics across multipart encoding and request transport selection so typed-nil readers are treated consistently as empty scalar fields.

Fixes #89.

Root cause

internal/apiform.encodeValue detected io.Reader capability before applying nil-pointer semantics. A typed-nil pointer therefore retained a concrete reader type and could reach io.Copy.

The request layer had the same semantic mismatch in a different place: inspectMultipartBody classified the same typed-nil reader as an upload even after the encoder treated it as an empty field. That unnecessarily selected one-shot streaming, disabled retries, and made an otherwise replayable scalar form reject 307/308 redirects.

Fix

• unwrap interface layers before reader detection in the multipart encoder and treat a resulting nil pointer as an empty field;
• classify typed-nil readers as scalar values before choosing buffered versus streamed multipart transport;
• keep typed-nil readers out of the unknown-size-reader path used for known-length upload framing, so they can coexist with regular file uploads without breaking Content-Length calculation;
• leave non-nil readers and real uploads on the existing streamed, non-retryable path.

Regression coverage

The tests cover:

• a concrete typed-nil reader stored in map[string]any;
• a typed-nil reader stored in map[string]io.Reader;
• a genuinely nil io.Reader interface;
• the established empty-field multipart representation;
• transport classification of a typed-nil reader as scalar/replayable;
• retry after an HTTP 429 while preserving the empty scalar field and buffered Content-Length;
• replay across both HTTP 307 and 308 redirects;
• a known-length file upload combined with a typed-nil optional reader, including exact-length streamed transport.

Validation

The earlier encoder-focused revision passed focused tests, race tests, repository compile-only tests, go mod verify, and lint locally. The transport follow-up is covered by the new request-level regressions above.

Fresh upstream CI, CodeQL, and Castiron runs are currently gated with action_required pending external-contributor approval.

Risk

Low. The behavior change is limited to typed-nil reader values. Non-nil readers and real uploads retain the existing streamed path with retries disabled; scalar-only forms remain buffered and replayable.

[jbeckwith-oai]

I followed up on the typed-nil review and pushed e6eeec6 via maintainer edits. The original shallow nil check still allowed a typed-nil pointer wrapped in an io.Reader-typed map or slice to reach io.Copy, because the outer interface itself was non-nil and implemented io.Reader. The encoder now unwraps interface layers before reader detection, checks the concrete pointer for nil, and removes the redundant interface/pointer branches. Regression coverage now includes map[string]any, map[string]io.Reader containing a typed-nil pointer, and a genuinely nil reader interface. Local validation passed: focused tests, package race tests, repository compile-only tests, go mod verify, diff checks, and the lint build (with VCS stamping disabled for this linked-worktree environment).

[HAYDEN-OAI]

Reviewed typed-nil reader handling across interface/pointer unwrapping, multipart field encoding, reader dispatch, header validation, and streamed request ownership/error propagation. No substantive correctness, security, or compatibility issues found.

[jbeckwith-oai]

Requesting changes because typed-nil reader normalization is inconsistent between multipart serialization and request transport selection. The encoder now writes a typed-nil reader as an empty scalar field, but inspectMultipartBody still classifies it as an upload and multipartRequestOptions therefore disables retries and selects a one-shot streamed body.

I reproduced this with a synthetic httptest server: a multipart request containing map[string]any{"file": (*reader)(nil)} received HTTP 429 and made only one attempt instead of retrying as a scalar-only form. Please normalize typed-nil readers consistently in the upload classifier and add an end-to-end request regression covering retry behavior. Redirect/replayability and Content-Length behavior should remain consistent with an empty scalar field as well.

The current-head CI, CodeQL, and Castiron custom-code workflows are also all stopped with action_required, so those checks still need to run successfully before merge.

[jbeckwith-oai]

[P2] Apply the same typed-nil semantics before choosing multipart transport. This branch correctly serializes a typed-nil reader as an empty scalar field, but pkg/cmd/multipartbody.go:inspectMultipartBody still sees the non-nil io.Reader interface and marks it as hasUpload. As a result, multipartRequestOptions unnecessarily selects one-shot streaming, applies option.WithMaxRetries(0), and rejects otherwise replayable 307/308 redirects. I verified the mismatch end to end with a synthetic HTTP 429 response: the empty-field request made one attempt and failed instead of retrying. Please update the upload classifier to recognize typed-nil readers as scalars and add a request-level retry regression.

[sylvesterkaczmarek]

Fixed. The transport classifier now applies the same typed-nil reader semantics as the encoder, so typed-nil readers stay on the buffered scalar path instead of being treated as uploads. I also updated the known-length framing path so a typed-nil optional reader can coexist with a real file upload.

Added request-level regressions covering the reported 429 retry case, replay across both 307 and 308 redirects, buffered Content-Length, the empty scalar-field representation, and a known-length file upload alongside a typed-nil reader.

Fresh CI, CodeQL and Castiron runs are currently action_required pending maintainer approval.