Hacktoberfest 2026:維護者為十月標記出來的 issue,仍然開放、適合新手。 瀏覽 Hacktoberfest issue

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

未關閉
#4,656 1 則留言 0 個 reaction 已指派 0 人 在 GitHub 檢視

維護者通常 1 天內回覆

@AdamMagued 已經在處理了。

開始於 2026年10月21日。

  • #4657 來自 @AdamMagued —— 未關閉
  • #4658 來自 @sai5880 —— 已關閉,未合併

評估

難度
3/5
預估耗時
1-2 天
新手友好度
70/100
Issue 類型
缺陷
描述清晰度
描述清楚
活躍度
活躍
技術堆疊
go

研究方向

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.

由索引模型根據 Issue 內容生成。

描述

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.
主要語言
Go
星號
11.3k
分支
2.6k
平均合併
2 天 8 小時
30 天內合併 PR
38

環境準備

  • 沒有 Dockerfile 或 Docker Compose 檔案
  • 沒有 Pull Request 範本
  • 閱讀貢獻指南

從這裡開始

  1. 先讀完整個 Issue,再讀專案的貢獻指南。
  2. 在 Issue 下留言說明你要接手 —— 這能避免兩個人做同樣的事。
  3. Fork 儲存庫,在一個分支上完成修改。
  4. 送出 Pull Request,並在描述裡引用這個 Issue 編號。

google/go-github 的其他 Issue

查看 google/go-github 的全部 Issue

相似的 Issue

更多 Go Issue

把新 issue 寄到你的電子郵件信箱

精選適合新手參與的 GitHub issue 摘要。