Stop LLM adapters following redirects and cap Retry-After
MQ-01: the Anthropic adapter used a default http.Client, so a 3xx from the configured endpoint was followed and the x-api-key header resent to the redirect target; net/http strips only its own sensitive headers. Both adapters now use a client whose CheckRedirect returns ErrUseLastResponse, so a 3xx comes back as an llm.ProviderError and nothing is requested again. The OpenAI-compatible adapter gets the same change: it forwards configured extra headers (which may carry a credential and which net/http does not treat as sensitive) across hosts, and net/http keeps Authorization on a same-host https-to-http redirect. MQ-02: a delay-seconds Retry-After too large for time.Duration overflowed into a negative wait. retryAfter now caps the value at the most whole seconds a time.Duration holds (also for values beyond int64) and the response still maps to ErrRateLimited. Tests: TestRedirectIsNotFollowed (302 to a second server, which must get no request) and TestRateLimitedHugeRetryAfterIsCapped in both packages, plus huge-number cases in TestRetryAfter. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -14,6 +14,7 @@ import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
"math"
|
||||
"net"
|
||||
"net/http"
|
||||
"net/url"
|
||||
@@ -94,7 +95,7 @@ func newFromConfig(cfg Config) (*Provider, error) {
|
||||
apiKey: cfg.APIKey,
|
||||
model: cfg.Model,
|
||||
maxTokens: cfg.MaxTokens,
|
||||
client: &http.Client{},
|
||||
client: noRedirectClient(),
|
||||
}, nil
|
||||
}
|
||||
|
||||
@@ -394,10 +395,15 @@ func retryAfter(value string, now time.Time) time.Duration {
|
||||
if value == "" {
|
||||
return 0
|
||||
}
|
||||
if secs, err := strconv.Atoi(value); err == nil {
|
||||
if secs, err := strconv.ParseInt(value, 10, 64); err == nil || errors.Is(err, strconv.ErrRange) {
|
||||
if secs < 0 {
|
||||
return 0
|
||||
}
|
||||
// time.Duration counts nanoseconds in an int64; a larger wait is
|
||||
// capped to the most whole seconds it holds instead of overflowing.
|
||||
if secs > maxRetryAfterSeconds {
|
||||
secs = maxRetryAfterSeconds
|
||||
}
|
||||
return time.Duration(secs) * time.Second
|
||||
}
|
||||
if at, err := http.ParseTime(value); err == nil && at.After(now) {
|
||||
@@ -406,6 +412,22 @@ func retryAfter(value string, now time.Time) time.Duration {
|
||||
return 0
|
||||
}
|
||||
|
||||
// maxRetryAfterSeconds is the largest delay-seconds value retryAfter
|
||||
// returns unchanged: the most whole seconds a time.Duration holds.
|
||||
const maxRetryAfterSeconds = math.MaxInt64 / int64(time.Second)
|
||||
|
||||
// noRedirectClient returns an HTTP client that never follows a redirect.
|
||||
// A followed redirect would resend the credential headers, which net/http
|
||||
// strips only for its own list of sensitive headers and only across hosts;
|
||||
// instead the 3xx response is returned and becomes a provider error.
|
||||
func noRedirectClient() *http.Client {
|
||||
return &http.Client{
|
||||
CheckRedirect: func(*http.Request, []*http.Request) error {
|
||||
return http.ErrUseLastResponse
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
func isTimeout(err error) bool {
|
||||
var ne net.Error
|
||||
return errors.Is(err, context.DeadlineExceeded) || (errors.As(err, &ne) && ne.Timeout())
|
||||
|
||||
@@ -647,6 +647,9 @@ func TestRetryAfter(t *testing.T) {
|
||||
"soon": 0,
|
||||
"Sat, 03 Oct 2026 12:02:00 GMT": 2 * time.Minute,
|
||||
"Sat, 03 Oct 2026 11:59:00 GMT": 0,
|
||||
"9223372036854775807": time.Duration(maxRetryAfterSeconds) * time.Second,
|
||||
"99999999999999999999": time.Duration(maxRetryAfterSeconds) * time.Second,
|
||||
"-99999999999999999999": 0,
|
||||
}
|
||||
for in, want := range cases {
|
||||
if got := retryAfter(in, now); got != want {
|
||||
@@ -910,3 +913,60 @@ func TestRedactedErrorsFormatWithoutKey(t *testing.T) {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestRateLimitedHugeRetryAfterIsCapped(t *testing.T) {
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||
w.Header().Set("Retry-After", "9223372036854775807")
|
||||
w.WriteHeader(http.StatusTooManyRequests)
|
||||
}))
|
||||
t.Cleanup(srv.Close)
|
||||
p, err := New(Config{BaseURL: srv.URL, APIKey: testKey, Model: "test-model", MaxTokens: 1024})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
err = p.Verify(context.Background())
|
||||
var rl *llm.RateLimitedError
|
||||
if !errors.Is(err, llm.ErrRateLimited) || !errors.As(err, &rl) || rl.RetryAfter != time.Duration(maxRetryAfterSeconds)*time.Second {
|
||||
t.Fatalf("err = %v, want a RateLimitedError with RetryAfter capped to %v", err, time.Duration(maxRetryAfterSeconds)*time.Second)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRedirectIsNotFollowed(t *testing.T) {
|
||||
var mu sync.Mutex
|
||||
var secondCalls int
|
||||
var secondHeader http.Header
|
||||
second := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
|
||||
mu.Lock()
|
||||
secondCalls++
|
||||
secondHeader = req.Header.Clone()
|
||||
mu.Unlock()
|
||||
w.WriteHeader(http.StatusOK)
|
||||
}))
|
||||
t.Cleanup(second.Close)
|
||||
first := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
|
||||
http.Redirect(w, req, second.URL+req.URL.Path, http.StatusFound)
|
||||
}))
|
||||
t.Cleanup(first.Close)
|
||||
p, err := New(Config{BaseURL: first.URL, APIKey: testKey, Model: "test-model", MaxTokens: 1024})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
err = p.Verify(context.Background())
|
||||
var pe *llm.ProviderError
|
||||
if !errors.Is(err, llm.ErrProvider) || !errors.As(err, &pe) || pe.Status() != http.StatusFound {
|
||||
t.Fatalf("err = %v, want a ProviderError for HTTP 302", err)
|
||||
}
|
||||
mu.Lock()
|
||||
defer mu.Unlock()
|
||||
if secondCalls != 0 {
|
||||
for name, values := range secondHeader {
|
||||
for _, v := range values {
|
||||
if strings.Contains(v, testKey) {
|
||||
t.Errorf("redirect target saw the key in %s", name)
|
||||
}
|
||||
}
|
||||
}
|
||||
t.Fatalf("redirect target got %d requests, want 0", secondCalls)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -15,6 +15,7 @@ import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
"math"
|
||||
"net"
|
||||
"net/http"
|
||||
"net/textproto"
|
||||
@@ -132,7 +133,7 @@ func newFromConfig(cfg Config) (*Provider, error) {
|
||||
model: cfg.Model,
|
||||
headers: headers,
|
||||
mode: cfg.StructuredOutput,
|
||||
client: &http.Client{},
|
||||
client: noRedirectClient(),
|
||||
}, nil
|
||||
}
|
||||
|
||||
@@ -462,10 +463,15 @@ func retryAfter(value string, now time.Time) time.Duration {
|
||||
if value == "" {
|
||||
return 0
|
||||
}
|
||||
if secs, err := strconv.Atoi(value); err == nil {
|
||||
if secs, err := strconv.ParseInt(value, 10, 64); err == nil || errors.Is(err, strconv.ErrRange) {
|
||||
if secs < 0 {
|
||||
return 0
|
||||
}
|
||||
// time.Duration counts nanoseconds in an int64; a larger wait is
|
||||
// capped to the most whole seconds it holds instead of overflowing.
|
||||
if secs > maxRetryAfterSeconds {
|
||||
secs = maxRetryAfterSeconds
|
||||
}
|
||||
return time.Duration(secs) * time.Second
|
||||
}
|
||||
if at, err := http.ParseTime(value); err == nil && at.After(now) {
|
||||
@@ -474,6 +480,22 @@ func retryAfter(value string, now time.Time) time.Duration {
|
||||
return 0
|
||||
}
|
||||
|
||||
// maxRetryAfterSeconds is the largest delay-seconds value retryAfter
|
||||
// returns unchanged: the most whole seconds a time.Duration holds.
|
||||
const maxRetryAfterSeconds = math.MaxInt64 / int64(time.Second)
|
||||
|
||||
// noRedirectClient returns an HTTP client that never follows a redirect.
|
||||
// A followed redirect would resend the credential headers, which net/http
|
||||
// strips only for its own list of sensitive headers and only across hosts;
|
||||
// instead the 3xx response is returned and becomes a provider error.
|
||||
func noRedirectClient() *http.Client {
|
||||
return &http.Client{
|
||||
CheckRedirect: func(*http.Request, []*http.Request) error {
|
||||
return http.ErrUseLastResponse
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
func isTimeout(err error) bool {
|
||||
var ne net.Error
|
||||
return errors.Is(err, context.DeadlineExceeded) || (errors.As(err, &ne) && ne.Timeout())
|
||||
|
||||
@@ -779,6 +779,9 @@ func TestRetryAfter(t *testing.T) {
|
||||
"soon": 0,
|
||||
"Sat, 03 Oct 2026 12:02:00 GMT": 2 * time.Minute,
|
||||
"Sat, 03 Oct 2026 11:59:00 GMT": 0,
|
||||
"9223372036854775807": time.Duration(maxRetryAfterSeconds) * time.Second,
|
||||
"99999999999999999999": time.Duration(maxRetryAfterSeconds) * time.Second,
|
||||
"-99999999999999999999": 0,
|
||||
}
|
||||
for in, want := range cases {
|
||||
if got := retryAfter(in, now); got != want {
|
||||
@@ -1142,3 +1145,60 @@ func TestRedactedErrorsFormatWithoutKey(t *testing.T) {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestRateLimitedHugeRetryAfterIsCapped(t *testing.T) {
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||
w.Header().Set("Retry-After", "9223372036854775807")
|
||||
w.WriteHeader(http.StatusTooManyRequests)
|
||||
}))
|
||||
t.Cleanup(srv.Close)
|
||||
p, err := New(Config{BaseURL: srv.URL + "/v1", APIKey: testKey, Model: "test-model", Headers: map[string]string{"X-Title": testKey}, StructuredOutput: PromptOnly})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
err = p.Verify(context.Background())
|
||||
var rl *llm.RateLimitedError
|
||||
if !errors.Is(err, llm.ErrRateLimited) || !errors.As(err, &rl) || rl.RetryAfter != time.Duration(maxRetryAfterSeconds)*time.Second {
|
||||
t.Fatalf("err = %v, want a RateLimitedError with RetryAfter capped to %v", err, time.Duration(maxRetryAfterSeconds)*time.Second)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRedirectIsNotFollowed(t *testing.T) {
|
||||
var mu sync.Mutex
|
||||
var secondCalls int
|
||||
var secondHeader http.Header
|
||||
second := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
|
||||
mu.Lock()
|
||||
secondCalls++
|
||||
secondHeader = req.Header.Clone()
|
||||
mu.Unlock()
|
||||
w.WriteHeader(http.StatusOK)
|
||||
}))
|
||||
t.Cleanup(second.Close)
|
||||
first := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
|
||||
http.Redirect(w, req, second.URL+req.URL.Path, http.StatusFound)
|
||||
}))
|
||||
t.Cleanup(first.Close)
|
||||
p, err := New(Config{BaseURL: first.URL + "/v1", APIKey: testKey, Model: "test-model", Headers: map[string]string{"X-Title": testKey}, StructuredOutput: PromptOnly})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
err = p.Verify(context.Background())
|
||||
var pe *llm.ProviderError
|
||||
if !errors.Is(err, llm.ErrProvider) || !errors.As(err, &pe) || pe.Status() != http.StatusFound {
|
||||
t.Fatalf("err = %v, want a ProviderError for HTTP 302", err)
|
||||
}
|
||||
mu.Lock()
|
||||
defer mu.Unlock()
|
||||
if secondCalls != 0 {
|
||||
for name, values := range secondHeader {
|
||||
for _, v := range values {
|
||||
if strings.Contains(v, testKey) {
|
||||
t.Errorf("redirect target saw the key in %s", name)
|
||||
}
|
||||
}
|
||||
}
|
||||
t.Fatalf("redirect target got %d requests, want 0", secondCalls)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user