Skip to content

Add support for HTTP_PROXY, HTTPS_PROXY, and NO_PROXY #1260

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
peterguy wants to merge 23 commits into
base: main
Choose a base branch
from
+710 −106
Open

Add support for HTTP_PROXY, HTTPS_PROXY, and NO_PROXY #1260

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
385a259
add function to read HTTP_PROXY/HTTPS_PROXY/NO_PROXY environment vari…
peterguy Feb 25, 2026
7651284
use new function to read proxy settings from environment, preferring …
peterguy Feb 25, 2026
d2ca262
use http.Request instead of manually building the request, and use Re…
peterguy Feb 25, 2026
d6b88e1
commit to a buffered reader for the proxy connection so we avoid nast…
peterguy Feb 25, 2026
d477d71
clone the transport TLS config instead of creating a new one, and mak…
peterguy Feb 25, 2026
662ff63
fix some comments
peterguy Feb 25, 2026
569629b
whoops, forgot to commit the io import
peterguy Feb 25, 2026
badd02e
dial the proxy, ensuring http/1.1 instead of http/2 for TLS-enabled p…
peterguy Feb 25, 2026
f3e4f82
add proxy tests
peterguy Feb 25, 2026
115b34b
change name of test to match production method
peterguy Feb 25, 2026
af15d4b
add 10ms delay to test proxy server startup to try to fix ubuntu tests
peterguy Feb 26, 2026
b83906c
go-lint.sh
peterguy Feb 26, 2026
f305cb6
wait for test proxy to startup
peterguy Feb 26, 2026
7977998
go-lint.sh
peterguy Feb 26, 2026
380249f
remove cleanEndpoint because it just obscures the one thing it does: …
peterguy Feb 27, 2026
0deaf78
parse the endpoint into a URL up front, and gather the proxy from the…
peterguy Feb 27, 2026
ac284e8
use the parsed endpoint url and consolidate proxy handling because th…
peterguy Feb 27, 2026
1bf5b79
fix proxyDialAddr and add more tests for it
peterguy Feb 27, 2026
43aa0c4
add EndpointURL to the tests and other places that should use it inst…
peterguy Feb 27, 2026
3536377
use the client to connect to the API instead of http.DefaultClient so…
peterguy Feb 27, 2026
419f6f7
go fmt ./...
keegancsmith Mar 2, 2026
76bfc55
fix golint issue
keegancsmith Mar 2, 2026
78eadd2
api: preserve hijack buffered bytes in proxy tests
keegancsmith Mar 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/src/login.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ Examples:
}

func loginCmd(ctx context.Context, cfg *config, client api.Client, endpointArg string, out io.Writer) error {
endpointArg = cleanEndpoint(endpointArg)
endpointArg = strings.TrimSuffix(endpointArg, "/")

printProblem := func(problem string) {
fmt.Fprintf(out, "❌ Problem: %s\n", problem)
Expand Down
7 changes: 5 additions & 2 deletions cmd/src/login_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"io"
"net/http"
"net/http/httptest"
"net/url"
"strings"
"testing"

Expand Down Expand Up @@ -63,7 +64,8 @@ func TestLogin(t *testing.T) {
defer s.Close()

endpoint := s.URL
out, err := check(t, &config{Endpoint: endpoint, AccessToken: "x"}, endpoint)
u, _ := url.ParseRequestURI(endpoint)
out, err := check(t, &config{Endpoint: endpoint, EndpointURL: u, AccessToken: "x"}, endpoint)
if err != cmderrors.ExitCode1 {
t.Fatal(err)
}
Expand All @@ -82,7 +84,8 @@ func TestLogin(t *testing.T) {
defer s.Close()

endpoint := s.URL
out, err := check(t, &config{Endpoint: endpoint, AccessToken: "x"}, endpoint)
u, _ := url.ParseRequestURI(endpoint)
out, err := check(t, &config{Endpoint: endpoint, EndpointURL: u, AccessToken: "x"}, endpoint)
if err != nil {
t.Fatal(err)
}
Expand Down
61 changes: 44 additions & 17 deletions cmd/src/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@ package main
import (
"encoding/json"
"flag"
"fmt"
"io"
"log"
"net"
"net/http"
"net/url"
"os"
"path/filepath"
Expand Down Expand Up @@ -107,6 +109,20 @@ func normalizeDashHelp(args []string) []string {
return args
}

func parseEndpoint(endpoint string) (*url.URL, error) {
u, err := url.ParseRequestURI(strings.TrimSuffix(endpoint, "/"))
if err != nil {
return nil, err
}
if !(u.Scheme == "http" || u.Scheme == "https") {
return nil, errors.Newf("Invalid scheme %s. Require http or https", u.Scheme)
}
if u.Host == "" {
return nil, errors.Newf("Empty host")
}
return u, nil
}

var cfg *config

// config represents the config format.
Copy link
Member

@keegancsmith keegancsmith Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Re: line +130]

I think you should marshal into a struct with just the json fields here. Then from your readConfig function return something which just contains the fields you want to read. Then you don't have both Endpoint and EndpointURL in this struct which is a bit weird.

See this comment inline on Graphite.

Expand All @@ -118,12 +134,13 @@ type config struct {
ProxyURL *url.URL
ProxyPath string
ConfigFilePath string
EndpointURL *url.URL
Comment on lines 134 to +137
Copy link
Member

@keegancsmith keegancsmith Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor: but these fields should probably all not be exported. Looks like we don't want to marshal them / they are internally read/set.

}

// apiClient returns an api.Client built from the configuration.
func (c *config) apiClient(flags *api.Flags, out io.Writer) api.Client {
return api.NewClient(api.ClientOpts{
Endpoint: c.Endpoint,
EndpointURL: c.EndpointURL,
AccessToken: c.AccessToken,
AdditionalHeaders: c.AdditionalHeaders,
Flags: flags,
Expand All @@ -133,7 +150,8 @@ func (c *config) apiClient(flags *api.Flags, out io.Writer) api.Client {
})
}

// readConfig reads the config file from the given path.
// readConfig reads the config from the standard config file, the (deprecated) user-specified config file,
// the environment variables, and the (deprecated) command-line flags.
func readConfig() (*config, error) {
cfgFile := *configPath
userSpecified := *configPath != ""
Expand Down Expand Up @@ -189,9 +207,21 @@ func readConfig() (*config, error) {
cfg.Proxy = envProxy
}

// Lastly, apply endpoint flag if set
if endpoint != nil && *endpoint != "" {
cfg.Endpoint = *endpoint
}

if endpointURL, err := parseEndpoint(cfg.Endpoint); err != nil {
return nil, errors.Newf("invalid endpoint: %s", cfg.Endpoint)
} else {
cfg.EndpointURL = endpointURL
cfg.Endpoint = endpointURL.String()
}

if cfg.Proxy != "" {

parseEndpoint := func(endpoint string) (scheme string, address string) {
parseProxyEndpoint := func(endpoint string) (scheme string, address string) {
parts := strings.SplitN(endpoint, "://", 2)
if len(parts) == 2 {
return parts[0], parts[1]
Expand All @@ -205,7 +235,7 @@ func readConfig() (*config, error) {
return slices.Contains(urlSchemes, scheme)
}

scheme, address := parseEndpoint(cfg.Proxy)
scheme, address := parseProxyEndpoint(cfg.Proxy)

if isURLScheme(scheme) {
endpoint := cfg.Proxy
Expand All @@ -227,11 +257,19 @@ func readConfig() (*config, error) {
return nil, errors.Newf("Invalid proxy configuration: %w", err)
}
if !isValidUDS {
return nil, errors.Newf("invalid proxy socket: %s", path)
return nil, errors.Newf("Invalid proxy socket: %s", path)
Copy link
Member

@keegancsmith keegancsmith Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you changed this is some other places as well. go convention is lowercase on error messages since they get combined. eg My outer error: My inner error doesn't read as nicely as my outer error: my inner error

}
cfg.ProxyPath = path
} else {
return nil, errors.Newf("invalid proxy endpoint: %s", cfg.Proxy)
return nil, errors.Newf("Invalid proxy endpoint: %s", cfg.Proxy)
}
} else {
// no SRC_PROXY; check for the standard proxy env variables HTTP_PROXY, HTTPS_PROXY, and NO_PROXY
if u, err := http.ProxyFromEnvironment(&http.Request{URL: cfg.EndpointURL}); err != nil {
// when there's an error, the value for the env variable is not a legit URL
return nil, fmt.Errorf("invalid HTTP_PROXY or HTTPS_PROXY value: %w", err)
} else {
cfg.ProxyURL = u
}
}

Expand All @@ -242,20 +280,9 @@ func readConfig() (*config, error) {
return nil, errConfigAuthorizationConflict
}

// Lastly, apply endpoint flag if set
if endpoint != nil && *endpoint != "" {
cfg.Endpoint = *endpoint
}

cfg.Endpoint = cleanEndpoint(cfg.Endpoint)

return &cfg, nil
}

func cleanEndpoint(urlStr string) string {
return strings.TrimSuffix(urlStr, "/")
}

// isValidUnixSocket checks if the given path is a valid Unix socket.
//
// Parameters:
Expand Down
96 changes: 80 additions & 16 deletions cmd/src/main_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,11 @@ func TestReadConfig(t *testing.T) {
{
name: "defaults",
want: &config{
Endpoint: "https://sourcegraph.com",
Endpoint: "https://sourcegraph.com",
EndpointURL: &url.URL{
Scheme: "https",
Host: "sourcegraph.com",
},
AdditionalHeaders: map[string]string{},
},
},
Expand All @@ -54,7 +58,11 @@ func TestReadConfig(t *testing.T) {
Proxy: "https://proxy.com:8080",
},
want: &config{
Endpoint: "https://example.com",
Endpoint: "https://example.com",
EndpointURL: &url.URL{
Scheme: "https",
Host: "example.com",
},
AccessToken: "deadbeef",
AdditionalHeaders: map[string]string{},
Proxy: "https://proxy.com:8080",
Expand Down Expand Up @@ -94,7 +102,11 @@ func TestReadConfig(t *testing.T) {
},
envProxy: "socks5://other.proxy.com:9999",
want: &config{
Endpoint: "https://example.com",
Endpoint: "https://example.com",
EndpointURL: &url.URL{
Scheme: "https",
Host: "example.com",
},
AccessToken: "deadbeef",
Proxy: "socks5://other.proxy.com:9999",
ProxyPath: "",
Expand All @@ -116,7 +128,11 @@ func TestReadConfig(t *testing.T) {
envEndpoint: "https://override.com",
envProxy: "socks5://other.proxy.com:9999",
want: &config{
Endpoint: "https://override.com",
Endpoint: "https://override.com",
EndpointURL: &url.URL{
Scheme: "https",
Host: "override.com",
},
AccessToken: "abc",
Proxy: "socks5://other.proxy.com:9999",
ProxyPath: "",
Expand All @@ -131,7 +147,11 @@ func TestReadConfig(t *testing.T) {
name: "no config file, token from environment",
envToken: "abc",
want: &config{
Endpoint: "https://sourcegraph.com",
Endpoint: "https://sourcegraph.com",
EndpointURL: &url.URL{
Scheme: "https",
Host: "sourcegraph.com",
},
AccessToken: "abc",
AdditionalHeaders: map[string]string{},
},
Expand All @@ -140,7 +160,11 @@ func TestReadConfig(t *testing.T) {
name: "no config file, endpoint from environment",
envEndpoint: "https://example.com",
want: &config{
Endpoint: "https://example.com",
Endpoint: "https://example.com",
EndpointURL: &url.URL{
Scheme: "https",
Host: "example.com",
},
AccessToken: "",
AdditionalHeaders: map[string]string{},
},
Expand All @@ -149,7 +173,11 @@ func TestReadConfig(t *testing.T) {
name: "no config file, proxy from environment",
envProxy: "https://proxy.com:8080",
want: &config{
Endpoint: "https://sourcegraph.com",
Endpoint: "https://sourcegraph.com",
EndpointURL: &url.URL{
Scheme: "https",
Host: "sourcegraph.com",
},
AccessToken: "",
Proxy: "https://proxy.com:8080",
ProxyPath: "",
Expand All @@ -166,7 +194,11 @@ func TestReadConfig(t *testing.T) {
envToken: "abc",
envProxy: "https://proxy.com:8080",
want: &config{
Endpoint: "https://example.com",
Endpoint: "https://example.com",
EndpointURL: &url.URL{
Scheme: "https",
Host: "example.com",
},
AccessToken: "abc",
Proxy: "https://proxy.com:8080",
ProxyPath: "",
Expand All @@ -181,7 +213,11 @@ func TestReadConfig(t *testing.T) {
name: "UNIX Domain Socket proxy using scheme and absolute path",
envProxy: "unix://" + socketPath,
want: &config{
Endpoint: "https://sourcegraph.com",
Endpoint: "https://sourcegraph.com",
EndpointURL: &url.URL{
Scheme: "https",
Host: "sourcegraph.com",
},
Proxy: "unix://" + socketPath,
ProxyPath: socketPath,
ProxyURL: nil,
Expand All @@ -192,7 +228,11 @@ func TestReadConfig(t *testing.T) {
name: "UNIX Domain Socket proxy with absolute path",
envProxy: socketPath,
want: &config{
Endpoint: "https://sourcegraph.com",
Endpoint: "https://sourcegraph.com",
EndpointURL: &url.URL{
Scheme: "https",
Host: "sourcegraph.com",
},
Proxy: socketPath,
ProxyPath: socketPath,
ProxyURL: nil,
Expand All @@ -203,7 +243,11 @@ func TestReadConfig(t *testing.T) {
name: "socks --> socks5",
envProxy: "socks://localhost:1080",
want: &config{
Endpoint: "https://sourcegraph.com",
Endpoint: "https://sourcegraph.com",
EndpointURL: &url.URL{
Scheme: "https",
Host: "sourcegraph.com",
},
Proxy: "socks://localhost:1080",
ProxyPath: "",
ProxyURL: &url.URL{
Expand All @@ -217,7 +261,11 @@ func TestReadConfig(t *testing.T) {
name: "socks5h",
envProxy: "socks5h://localhost:1080",
want: &config{
Endpoint: "https://sourcegraph.com",
Endpoint: "https://sourcegraph.com",
EndpointURL: &url.URL{
Scheme: "https",
Host: "sourcegraph.com",
},
Proxy: "socks5h://localhost:1080",
ProxyPath: "",
ProxyURL: &url.URL{
Expand All @@ -236,7 +284,11 @@ func TestReadConfig(t *testing.T) {
AdditionalHeaders: map[string]string{},
},
want: &config{
Endpoint: "https://override.com",
Endpoint: "https://override.com",
EndpointURL: &url.URL{
Scheme: "https",
Host: "override.com",
},
AccessToken: "deadbeef",
AdditionalHeaders: map[string]string{},
},
Expand All @@ -247,7 +299,11 @@ func TestReadConfig(t *testing.T) {
envEndpoint: "https://example.com",
envToken: "abc",
want: &config{
Endpoint: "https://override.com",
Endpoint: "https://override.com",
EndpointURL: &url.URL{
Scheme: "https",
Host: "override.com",
},
AccessToken: "abc",
AdditionalHeaders: map[string]string{},
},
Expand All @@ -259,7 +315,11 @@ func TestReadConfig(t *testing.T) {
envToken: "abc",
envFooHeader: "bar",
want: &config{
Endpoint: "https://override.com",
Endpoint: "https://override.com",
EndpointURL: &url.URL{
Scheme: "https",
Host: "override.com",
},
AccessToken: "abc",
AdditionalHeaders: map[string]string{"foo": "bar"},
},
Expand All @@ -271,7 +331,11 @@ func TestReadConfig(t *testing.T) {
envToken: "abc",
envHeaders: "foo:bar\nfoo-bar:bar-baz",
want: &config{
Endpoint: "https://override.com",
Endpoint: "https://override.com",
EndpointURL: &url.URL{
Scheme: "https",
Host: "override.com",
},
Comment on lines +335 to +338
Copy link
Member

@keegancsmith keegancsmith Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it worth updating all these test cases for this? I'd consider just not comparing this field in the comparsion in the test.

AccessToken: "abc",
AdditionalHeaders: map[string]string{"foo-bar": "bar-baz", "foo": "bar"},
},
Expand Down
Loading
Loading