Rate-limit retry replays a consumed request body for POST/PUT/PATCH #4656

Summary

The automatic primary rate-limit retry path in Client.bareDo re-sends a cloned *http.Request without restoring its body. For any request that carries a body (POST, PUT, PATCH), the original req.Body has already been consumed by the first caller.Do(req) call, so the retry sends an empty body or fails on ContentLength.

Location

github/github.go:1521-1529 (retry branch in bareDo), with request construction at github/github.go:993-1037 (NewRequest) and the send at github/github.go:1464 (caller.Do(req)).

var rateLimitError *RateLimitError
if errors.As(err, &rateLimitError) &&
    ctx.Value(SleepUntilPrimaryRateLimitResetWhenRateLimited) != nil {
    if err := sleepUntilResetWithBuffer(ctx, rateLimitError.Rate.Reset.Time); err != nil {
        return response, err
    }
    // retry the request now the rate limit should have been reset
    newReq := req.Clone(context.WithValue(ctx, SleepUntilPrimaryRateLimitResetWhenRateLimited, nil))
    return c.bareDo(caller, newReq)
}

Details

NewRequest encodes the body into a *bytes.Buffer:

var buf io.ReadWriter
if body != nil {
    buf = &bytes.Buffer{}
    enc := json.NewEncoder(buf)
    enc.SetEscapeHTML(false)
    err := enc.Encode(body)
    if err != nil {
        return nil, err
    }
}

req, err := http.NewRequestWithContext(ctx, method, u.String(), buf)

For the *bytes.Buffer case, net/http.NewRequestWithContext populates req.GetBody and sets req.ContentLength. However, bareDo never uses GetBody. It calls caller.Do(req) at line 1464, which reads req.Body to EOF (and closes it). When a *RateLimitError triggers the retry branch, req.Clone(...) performs a shallow copy: the cloned request keeps the same consumed *http.Request.Body reader and the original ContentLength. The retry therefore either sends a zero-length body while still advertising the original ContentLength, or returns an error such as http: ContentLength=N with Body length 0.

The fix belongs in this retry path (or in bareDo before every send): call req.GetBody() and assign the resulting reader to req.Body before re-sending. If GetBody is nil (a body that cannot be replayed, for example a stream passed to NewFormRequest), the retry must not be attempted, because there is no way to reconstruct the payload.

Impact

• Users who opt into SleepUntilPrimaryRateLimitResetWhenRateLimited and then perform a write operation (POST/PUT/PATCH) can have that write silently truncated to an empty body after a primary rate-limit retry. This is a correctness issue and can cause confusing API errors or, depending on the endpoint, unintended empty updates.
• The retry can also fail outright with a ContentLength mismatch error instead of succeeding on the second attempt.
• GET requests are unaffected, which is why existing tests do not detect this.

Suggested fix

In the retry branch, restore the body from GetBody before cloning or re-sending:

newReq := req.Clone(context.WithValue(ctx, SleepUntilPrimaryRateLimitResetWhenRateLimited, nil))
if req.GetBody != nil {
    body, err := req.GetBody()
    if err != nil {
        return response, err
    }
    newReq.Body = body
} else if req.Body != nil && req.Body != http.NoBody {
    // Body cannot be replayed; do not retry.
    return response, err
}
return c.bareDo(caller, newReq)

Applying the restore inside bareDo before each caller.Do would also protect other re-send paths, but the minimal change is the rate-limit retry branch.

Verification

• Static review of github/github.go at HEAD 2190e6a (2026-10-22): NewRequest encodes into bytes.Buffer, bareDo calls caller.Do once and then req.Clone on retry, and no GetBody restoration exists in github/github.go.
• Existing retry coverage is GET-only. The SleepUntilPrimaryRateLimitResetWhenRateLimited tests in github/github_test.go (lines 3033, 3066, 3095, 3150, 3201) all construct requests with "GET" and nil body, so they never exercise body replay.
• Suggested regression tests: a table test using POST, PUT, and PATCH where the handler returns a *RateLimitError (403 with rate-limit headers) on the first call and 200 on the second, asserting the second request receives the original JSON body. Add a case for a non-replayable body to assert the retry is skipped.
• This report is based on static source review. No runtime proof of concept was executed.