Skip to content
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.

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

Configurable http and https proxy addrs #22

Merged
merged 2 commits into from
Nov 13, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
11 changes: 8 additions & 3 deletions https.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ func (proxy *ProxyHttpServer) handleHttps(w http.ResponseWriter, r *http.Request
host += ":80"
}

httpsProxy, err := httpsProxyFromEnv(r.URL)
httpsProxy, err := httpsProxy(r.URL, proxy.HttpsProxyAddr)
if err != nil {
ctx.Warnf("Error configuring HTTPS proxy err=%q url=%q", err, r.URL.String())
}
Expand Down Expand Up @@ -559,10 +559,15 @@ func (proxy *ProxyHttpServer) connectDialProxyWithContext(ctx *ProxyCtx, proxyHo
return c, nil
}

// httpsProxyFromEnv allows goproxy to respect no_proxy env vars
// httpsProxy allows goproxy to respect no_proxy env vars

Choose a reason for hiding this comment

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

Could you extend this comment to clarify which configuration takes precedent?

// https://github.com/stripe/goproxy/pull/5
func httpsProxyFromEnv(reqURL *url.URL) (string, error) {
func httpsProxy(reqURL *url.URL, httpProxyAddr string) (string, error) {

Choose a reason for hiding this comment

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

Could you change the var name to httpsProxyAddr to be more accurate?

cfg := httpproxy.FromEnvironment()

if httpProxyAddr != "" {
cfg.HTTPSProxy = httpProxyAddr
}

// We only use this codepath for HTTPS CONNECT proxies so we shouldn't
// return anything from HTTPProxy
cfg.HTTPProxy = ""
Expand Down
32 changes: 17 additions & 15 deletions https_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,24 +7,26 @@ import (
)

var proxytests = map[string]struct {
noProxy string
httpsProxy string
url string
expectProxy string
noProxy string
envHttpsProxy string
customHttpsProxy string
url string
expectProxy string
}{
"do not proxy without a proxy configured": {"", "", "https://foo.bar/baz", ""},
"proxy with a proxy configured": {"", "daproxy", "https://foo.bar/baz", "http://daproxy:http"},
"proxy without a scheme": {"", "daproxy", "//foo.bar/baz", "http://daproxy:http"},
"proxy with a proxy configured with a port": {"", "http://daproxy:123", "https://foo.bar/baz", "http://daproxy:123"},
"proxy with an https proxy configured": {"", "https://daproxy", "https://foo.bar/baz", "https://daproxy:https"},
"proxy with a non-matching no_proxy": {"other.bar", "daproxy", "https://foo.bar/baz", "http://daproxy:http"},
"do not proxy with a full no_proxy match": {"foo.bar", "daproxy", "https://foo.bar/baz", ""},
"do not proxy with a suffix no_proxy match": {".bar", "daproxy", "https://foo.bar/baz", ""},
"do not proxy without a proxy configured": {"", "", "", "https://foo.bar/baz", ""},
"proxy with a proxy configured": {"", "daproxy", "", "https://foo.bar/baz", "http://daproxy:http"},
"proxy without a scheme": {"", "daproxy", "", "//foo.bar/baz", "http://daproxy:http"},
"proxy with a proxy configured with a port": {"", "http://daproxy:123", "", "https://foo.bar/baz", "http://daproxy:123"},
"proxy with an https proxy configured": {"", "https://daproxy", "", "https://foo.bar/baz", "https://daproxy:https"},
"proxy with a non-matching no_proxy": {"other.bar", "daproxy", "", "https://foo.bar/baz", "http://daproxy:http"},
"do not proxy with a full no_proxy match": {"foo.bar", "daproxy", "", "https://foo.bar/baz", ""},
"do not proxy with a suffix no_proxy match": {".bar", "daproxy", "", "https://foo.bar/baz", ""},
"proxy with an custom https proxy": {"", "https://daproxy", "https://customproxy", "https://foo.bar/baz", "https://customproxy:https"},
}

var envKeys = []string{"no_proxy", "http_proxy", "https_proxy", "NO_PROXY", "HTTP_PROXY", "HTTPS_PROXY"}

func TestHttpsProxyFromEnv(t *testing.T) {
func TestHttpsProxy(t *testing.T) {
for _, k := range envKeys {
v, ok := os.LookupEnv(k)
if ok {
Expand All @@ -43,14 +45,14 @@ func TestHttpsProxyFromEnv(t *testing.T) {
for name, spec := range proxytests {
t.Run(name, func(t *testing.T) {
os.Setenv("no_proxy", spec.noProxy)
os.Setenv("https_proxy", spec.httpsProxy)
os.Setenv("https_proxy", spec.envHttpsProxy)

url, err := url.Parse(spec.url)
if err != nil {
t.Fatalf("bad test input URL %s: %v", spec.url, err)
}

actual, err := httpsProxyFromEnv(url)
actual, err := httpsProxy(url, spec.customHttpsProxy)
if err != nil {
t.Fatalf("unexpected error parsing proxy from env: %#v", err)
}
Expand Down
62 changes: 60 additions & 2 deletions proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,12 @@ import (
"log"
"net"
"net/http"
"net/url"
"os"
"regexp"
"sync/atomic"

"golang.org/x/net/http/httpproxy"
)

// The basic proxy type. Implements http.Handler.
Expand Down Expand Up @@ -54,6 +57,10 @@ type ProxyHttpServer struct {
// ConnectRespHandler allows users to mutate the response to the CONNECT request before it
// is returned to the client.
ConnectRespHandler func(ctx *ProxyCtx, resp *http.Response) error

// HTTP and HTTPS proxy addresses
HttpProxyAddr string
HttpsProxyAddr string
}

var hasPort = regexp.MustCompile(`:\d+$`)
Expand Down Expand Up @@ -179,8 +186,40 @@ func (proxy *ProxyHttpServer) ServeHTTP(w http.ResponseWriter, r *http.Request)
}
}

type options struct {
httpProxyAddr string
httpsProxyAddr string
}
type fnOption func(*options)

func (fn fnOption) apply(opts *options) { fn(opts) }

type ProxyHttpServerOptions interface {
apply(*options)
}

func WithHttpProxyAddr(httpProxyAddr string) ProxyHttpServerOptions {
return fnOption(func(opts *options) {
opts.httpProxyAddr = httpProxyAddr
})
}

func WithHttpsProxyAddr(httpsProxyAddr string) ProxyHttpServerOptions {
return fnOption(func(opts *options) {
opts.httpsProxyAddr = httpsProxyAddr
})
}

// NewProxyHttpServer creates and returns a proxy server, logging to stderr by default
func NewProxyHttpServer() *ProxyHttpServer {
func NewProxyHttpServer(opts ...ProxyHttpServerOptions) *ProxyHttpServer {
appliedOpts := &options{
httpProxyAddr: "",
httpsProxyAddr: "",
}
for _, opt := range opts {
opt.apply(appliedOpts)
}

proxy := ProxyHttpServer{
Logger: log.New(os.Stderr, "", log.LstdFlags),
reqHandlers: []ReqHandler{},
Expand All @@ -191,7 +230,26 @@ func NewProxyHttpServer() *ProxyHttpServer {
}),
Tr: &http.Transport{TLSClientConfig: tlsClientSkipVerify, Proxy: http.ProxyFromEnvironment},
}
proxy.ConnectDial = dialerFromEnv(&proxy)

cfg := httpproxy.FromEnvironment()

Choose a reason for hiding this comment

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

I'm not sure about silently overriding environment proxy variables. WDYT?

Choose a reason for hiding this comment

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

Also could we name this something to signal what it actually is instead of just cfg?

if appliedOpts.httpProxyAddr != "" {
proxy.HttpProxyAddr = appliedOpts.httpProxyAddr
cfg.HTTPProxy = appliedOpts.httpProxyAddr
}

if appliedOpts.httpsProxyAddr != "" {
proxy.HttpsProxyAddr = appliedOpts.httpsProxyAddr
cfg.HTTPSProxy = appliedOpts.httpsProxyAddr
proxy.ConnectDial = proxy.NewConnectDialToProxy(appliedOpts.httpsProxyAddr)
} else {
proxy.ConnectDial = dialerFromEnv(&proxy)
}

if appliedOpts.httpProxyAddr != "" || appliedOpts.httpsProxyAddr != "" {
proxy.Tr.Proxy = func(req *http.Request) (*url.URL, error) {
return cfg.ProxyFunc()(req.URL)
}
}

return &proxy
}
50 changes: 50 additions & 0 deletions proxy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -703,7 +703,57 @@ func TestGoproxyThroughProxy(t *testing.T) {
if r := string(getOrFail(https.URL+"/bobo", client, t)); r != "bobo bobo" {
t.Error("Expected bobo doubled twice, got", r)
}
}

func TestHttpProxyAddrsFromEnv(t *testing.T) {
proxy := goproxy.NewProxyHttpServer()
doubleString := func(resp *http.Response, ctx *goproxy.ProxyCtx) *http.Response {
b, err := ioutil.ReadAll(resp.Body)
panicOnErr(err, "readAll resp")
resp.Body = ioutil.NopCloser(bytes.NewBufferString(string(b) + " " + string(b)))
return resp
}
proxy.OnRequest().HandleConnect(goproxy.AlwaysMitm)
proxy.OnResponse().DoFunc(doubleString)

_, l := oneShotProxy(proxy, t)
defer l.Close()

os.Setenv("http_proxy", l.URL)
os.Setenv("https_proxy", l.URL)
proxy2 := goproxy.NewProxyHttpServer()

client, l2 := oneShotProxy(proxy2, t)
defer l2.Close()
if r := string(getOrFail(https.URL+"/bobo", client, t)); r != "bobo bobo" {
t.Error("Expected bobo doubled twice, got", r)
}

os.Unsetenv("http_proxy")
os.Unsetenv("https_proxy")
}

func TestCustomHttpProxyAddrs(t *testing.T) {
proxy := goproxy.NewProxyHttpServer()
doubleString := func(resp *http.Response, ctx *goproxy.ProxyCtx) *http.Response {
b, err := ioutil.ReadAll(resp.Body)
panicOnErr(err, "readAll resp")
resp.Body = ioutil.NopCloser(bytes.NewBufferString(string(b) + " " + string(b)))
return resp
}
proxy.OnRequest().HandleConnect(goproxy.AlwaysMitm)
proxy.OnResponse().DoFunc(doubleString)

_, l := oneShotProxy(proxy, t)
defer l.Close()

proxy2 := goproxy.NewProxyHttpServer(goproxy.WithHttpProxyAddr(l.URL), goproxy.WithHttpsProxyAddr(l.URL))

client, l2 := oneShotProxy(proxy2, t)
defer l2.Close()
if r := string(getOrFail(https.URL+"/bobo", client, t)); r != "bobo bobo" {
t.Error("Expected bobo doubled twice, got", r)
}
}

func TestGoproxyHijackConnect(t *testing.T) {
Expand Down
Loading