openai-go icon indicating copy to clipboard operation
openai-go copied to clipboard

WithBaseURL is not concurrency-safe, causing invalid URLs during concurrent calls

Open zenxme opened this issue 4 months ago • 1 comments

Description

The WithBaseURL function is not safe for concurrent use. When called from multiple goroutines simultaneously, it may generate URLs with double slashes (//) due to non-atomic path manipulation.

Reproduction Code

package main

import (
    "context"
    "fmt"
    "net/http"
    "sync"

    openai "github.com/openai/openai-go"
    "github.com/openai/openai-go/option"
)

func main() {
    var (
        mu       sync.Mutex
        urlToCnt = make(map[string]int)
    )
    for range 1000 {
        client := openai.NewClient(
            option.WithBaseURL("http://111/v1"),
            option.WithMaxRetries(0),
            option.WithMiddleware(func(req *http.Request, next option.MiddlewareNext) (*http.Response, error) {
                mu.Lock()
                defer mu.Unlock()
                urlToCnt[req.URL.Path] += 1
                return next(req)
            }),
        )
        var wg sync.WaitGroup
        for range 1000 {
            wg.Add(1)
            go func() {
                defer wg.Done()
                _, _ = client.Chat.Completions.New(context.TODO(), openai.ChatCompletionNewParams{})
            }()
        }
        wg.Wait()
        fmt.Println(urlToCnt)
    }
    fmt.Println(urlToCnt)
}

Observed Behavior

Running this code produces invalid paths containing double slashes:

map[/v1//chat/completions:9000 /v1/chat/completions:991000]
......

Note the invalid /v1//chat/completions path.

Root Cause

The path normalization in WithBaseURL lacks synchronization:

func WithBaseURL(base string) RequestOption {
    u, err := url.Parse(base)
    return requestconfig.RequestOptionFunc(func(r *requestconfig.RequestConfig) error {
        if err != nil {
            return fmt.Errorf("requestoption: WithBaseURL failed to parse url %s\n", err)
        }

        if u.Path != "" && !strings.HasSuffix(u.Path, "/") {
            u.Path += "/"  // ❌ Non-atomic read-modify-write
        }
        r.BaseURL = u
        return nil
    })
}

The check strings.HasSuffix(u.Path, "/") and subsequent u.Path += "/" are not atomic. Concurrent calls may cause:

  • Multiple goroutines to see u.Path without trailing slash
  • Multiple appends of /, creating // in the path

zenxme avatar Jul 15 '25 02:07 zenxme

Thanks for this raising this detailed issue! I'll look into a fix

jacobzim-stl avatar Jul 17 '25 15:07 jacobzim-stl