Hacktoberfest 2026: những issue maintainer đã đánh dấu cho tháng Mười, đang mở và phù hợp người mới. Xem issue Hacktoberfest

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

Đang mở
  • Có gì không ổn?
#4,656 1 bình luận 0 reaction 0 người được giao Xem trên GitHub

Maintainer thường phản hồi trong vòng 1 ngày

@AdamMagued đang làm issue này rồi.

Từ ngày 21/10/2026.

  • #4657 của @AdamMagued — đang mở
  • #4658 của @sai5880 — đã đóng, không merge

Đánh giá

Độ khó
3/5
Thời gian dự kiến
1-2 ngày
Mức phù hợp với người mới
70/100
Loại issue
Lỗi
Độ rõ ràng
Đặc tả rõ ràng
Mức độ hoạt động
Sôi nổi
Công nghệ
go
Lĩnh vực
backend-api-design

Hướng nghiên cứu

Start with the retry branch in github/github.go at lines 1521–1529, then read NewRequest and the existing retry tests in github/github_test.go named in the issue. Add regression coverage for POST, PUT, and PATCH bodies and for a non-replayable body. Done means retry requests preserve replayable bodies and requests without a replayable body are not retried.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Mô tả

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.
Ngôn ngữ chính
Go
Star
11.3k
Fork
2.6k
Merge trung bình
2 ngày 8 giờ
Pull request đã merge (30 ngày)
38

Chuẩn bị môi trường

Bắt đầu từ đâu

  1. Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
  2. Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
  3. Fork repository và làm thay đổi trên một nhánh.
  4. Mở pull request có tham chiếu số hiệu của issue.

Issue khác của google/go-github

Tất cả issue của google/go-github

Issue tương tự

Thêm issue về Go

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.