diff --git a/github/client.go b/github/client.go index d5ffbc0..9eaad24 100644 --- a/github/client.go +++ b/github/client.go @@ -10,7 +10,6 @@ import ( "io" "log/slog" "net/http" - "net/url" "os" "strconv" "strings" @@ -108,16 +107,6 @@ func AllowInsecureHTTP() ClientOption { } } -// AllowInsecureHTTPForTest permits the client to send credentials over HTTP -// without requiring the REVIEW_BOT_ALLOW_INSECURE environment variable. -// This is intended exclusively for tests using httptest.Server. -func AllowInsecureHTTPForTest() ClientOption { - return func(c *clientConfig) { - c.allowInsecureHTTP = true - c.testBypass = true - } -} - // Client interacts with the GitHub API. // A Client is safe for concurrent use by multiple goroutines. // SetHTTPClient and SetRetryBackoff are intended for test setup only and must @@ -173,7 +162,7 @@ func defaultCheckRedirect(req *http.Request, via []*http.Request) error { // NewClient creates a new GitHub API client. // If baseURL is empty, it defaults to https://api.github.com. // For GitHub Enterprise, pass the API base URL (e.g. https://github.concur.com/api/v3). -// The baseURL must use HTTPS unless AllowInsecureHTTP() or AllowInsecureHTTPForTest() +// The baseURL must use HTTPS unless AllowInsecureHTTP() or AllowInsecureHTTPForTest() (test-only) // is passed as an option. func NewClient(token, baseURL string, opts ...ClientOption) *Client { if baseURL == "" { @@ -273,17 +262,21 @@ func (c *Client) parseRetryAfter(value string) (time.Duration, bool) { return 0, false } +// hasHTTPSScheme reports whether rawURL starts with "https://" (case-insensitive). +// It avoids the allocation of url.Parse for a simple scheme check. +func hasHTTPSScheme(rawURL string) bool { + const prefix = "https://" + return len(rawURL) >= len(prefix) && strings.EqualFold(rawURL[:len(prefix)], prefix) +} + // doRequest performs an HTTP request with retry on 429 rate limit responses. // It respects the Retry-After header when present, supporting both integer // seconds and HTTP-date formats (capped at maxRetryAfter). func (c *Client) doRequest(ctx context.Context, method, reqURL string, accept string) ([]byte, error) { // Reject non-HTTPS URLs when credentials are present and insecure mode is not enabled. + // Uses a string prefix check instead of url.Parse to avoid per-request allocations. if c.token != "" && !c.allowInsecureHTTP { - parsed, err := url.Parse(reqURL) - if err != nil { - return nil, fmt.Errorf("parse request URL: %w", err) - } - if !strings.EqualFold(parsed.Scheme, "https") { + if !hasHTTPSScheme(reqURL) { return nil, fmt.Errorf("refusing to send credentials over non-HTTPS URL %q (use AllowInsecureHTTP option for trusted networks)", reqURL) } } diff --git a/github/export_test.go b/github/export_test.go new file mode 100644 index 0000000..a609c84 --- /dev/null +++ b/github/export_test.go @@ -0,0 +1,14 @@ +package github + +// AllowInsecureHTTPForTest permits the client to send credentials over HTTP +// without requiring the REVIEW_BOT_ALLOW_INSECURE environment variable. +// This is intended exclusively for tests using httptest.Server. +// +// This function lives in export_test.go so it is only available to test +// binaries and does not appear in the production API surface. +func AllowInsecureHTTPForTest() ClientOption { + return func(c *clientConfig) { + c.allowInsecureHTTP = true + c.testBypass = true + } +}