From 38bb05c6c6b99dce2d543df7183a92817c8d0caa Mon Sep 17 00:00:00 2001 From: Mohammed Al Sahaf Date: Mon, 26 Aug 2024 17:46:27 +0000 Subject: [PATCH] address feedbcak --- .../reverse_proxy_http_transport_none_proxy.txt | 2 +- .../reverse_proxy_http_transport_url_proxy.txt | 2 +- modules/caddyhttp/reverseproxy/caddyfile.go | 17 +++++++++-------- modules/caddyhttp/reverseproxy/httptransport.go | 17 ++++++++++++++++- modules/internal/network/networkproxy.go | 10 ++++++---- 5 files changed, 33 insertions(+), 15 deletions(-) diff --git a/caddytest/integration/caddyfile_adapt/reverse_proxy_http_transport_none_proxy.txt b/caddytest/integration/caddyfile_adapt/reverse_proxy_http_transport_none_proxy.txt index 6d41ed5b..3805448d 100644 --- a/caddytest/integration/caddyfile_adapt/reverse_proxy_http_transport_none_proxy.txt +++ b/caddytest/integration/caddyfile_adapt/reverse_proxy_http_transport_none_proxy.txt @@ -1,7 +1,7 @@ :8884 reverse_proxy 127.0.0.1:65535 { transport http { - forward_proxy none + network_proxy none } } ---------- diff --git a/caddytest/integration/caddyfile_adapt/reverse_proxy_http_transport_url_proxy.txt b/caddytest/integration/caddyfile_adapt/reverse_proxy_http_transport_url_proxy.txt index c4134eec..9397458e 100644 --- a/caddytest/integration/caddyfile_adapt/reverse_proxy_http_transport_url_proxy.txt +++ b/caddytest/integration/caddyfile_adapt/reverse_proxy_http_transport_url_proxy.txt @@ -1,7 +1,7 @@ :8884 reverse_proxy 127.0.0.1:65535 { transport http { - forward_proxy url http://localhost:8080 + network_proxy url http://localhost:8080 } } ---------- diff --git a/modules/caddyhttp/reverseproxy/caddyfile.go b/modules/caddyhttp/reverseproxy/caddyfile.go index 9fe6a1eb..59a7ff75 100644 --- a/modules/caddyhttp/reverseproxy/caddyfile.go +++ b/modules/caddyhttp/reverseproxy/caddyfile.go @@ -980,7 +980,9 @@ func (h *Handler) FinalizeUnmarshalCaddyfile(helper httpcaddyfile.Helper) error // read_buffer // write_buffer // max_response_header -// forward_proxy_url +// network_proxy { +// ... +// } // dial_timeout // dial_fallback_delay // response_header_timeout @@ -991,6 +993,9 @@ func (h *Handler) FinalizeUnmarshalCaddyfile(helper httpcaddyfile.Helper) error // tls_insecure_skip_verify // tls_timeout // tls_trusted_ca_certs +// tls_trust_pool { +// ... +// } // tls_server_name // tls_renegotiation // tls_except_ports @@ -1069,12 +1074,13 @@ func (h *HTTPTransport) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { } case "forward_proxy_url": + caddy.Log().Warn("The 'forward_proxy_url' field is deprecated. Use the 'network_proxy' field instead.") if !d.NextArg() { return d.ArgErr() } - h.ForwardProxyURL = d.Val() - case "forward_proxy": + h.ForwardProxyURL = d.Val() + case "network_proxy": if !d.NextArg() { return d.ArgErr() } @@ -1084,11 +1090,6 @@ func (h *HTTPTransport) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { if err != nil { return err } - // TODO: should we validate here? - // ca, ok := unm.(caddy.ProxyFuncProducer) - // if !ok { - // return d.Errf("module %s is not a caddy.ProxyFuncProducer", modID) - // } h.NetworkProxyRaw = caddyconfig.JSONModuleObject(unm, "from", modStem, nil) case "dial_timeout": if !d.NextArg() { diff --git a/modules/caddyhttp/reverseproxy/httptransport.go b/modules/caddyhttp/reverseproxy/httptransport.go index 55b25a48..e9e31b1c 100644 --- a/modules/caddyhttp/reverseproxy/httptransport.go +++ b/modules/caddyhttp/reverseproxy/httptransport.go @@ -88,6 +88,7 @@ type HTTPTransport struct { // forward_proxy_url -> upstream // // Default: http.ProxyFromEnvironment + // DEPRECATED: Use NetworkProxyRaw|`network_proxy` instead. Subject to removal. ForwardProxyURL string `json:"forward_proxy_url,omitempty"` // How long to wait before timing out trying to connect to @@ -139,7 +140,20 @@ type HTTPTransport struct { // The pre-configured underlying HTTP transport. Transport *http.Transport `json:"-"` - // Forward proxy module + // The module that provides the network (forward) proxy + // URL that the HTTP transport will use to proxy + // requests to the upstream. See [http.Transport.Proxy](https://pkg.go.dev/net/http#Transport.Proxy) + // for information regarding supported protocols. + // + // Providing a value to this parameter results in + // requests flowing through the reverse_proxy in the following + // way: + // + // User Agent -> + // reverse_proxy -> + // [proxy provided by the module] -> upstream + // + // If nil/empty, default to reading the `HTTP_PROXY`, `HTTPS_PROXY`, and `NO_PROXY`. NetworkProxyRaw json.RawMessage `json:"network_proxy,omitempty" caddy:"namespace=caddy.network_proxy.source inline_key=from"` h2cTransport *http2.Transport @@ -343,6 +357,7 @@ func (h *HTTPTransport) NewTransport(caddyCtx caddy.Context) (*http.Transport, e } if h.ForwardProxyURL != "" { + caddyCtx.Logger().Warn("forward_proxy_url is deprecated; use network_proxy instead") pUrl, err := url.Parse(h.ForwardProxyURL) if err != nil { return nil, fmt.Errorf("failed to parse transport proxy url: %v", err) diff --git a/modules/internal/network/networkproxy.go b/modules/internal/network/networkproxy.go index f75168e8..fa0f5009 100644 --- a/modules/internal/network/networkproxy.go +++ b/modules/internal/network/networkproxy.go @@ -17,6 +17,7 @@ func init() { caddy.RegisterModule(ProxyFromNone{}) } +// The "url" proxy source uses the defined URL as the proxy type ProxyFromURL struct { URL string `json:"url"` @@ -65,17 +66,17 @@ func (p ProxyFromURL) ProxyFunc() func(*http.Request) (*url.URL, error) { // note: h.ForwardProxyURL should never be empty at this point s := repl.ReplaceAll(p.URL, "") if s == "" { - p.logger.Error("forward_proxy_url was empty after applying placeholders", + p.logger.Error("network_proxy URL was empty after applying placeholders", zap.String("initial_value", p.URL), zap.String("final_value", s), zap.String("hint", "check for invalid placeholders")) - return nil, errors.New("empty value for forward_proxy_url") + return nil, errors.New("empty value for network_proxy URL") } // parse the url pUrl, err := url.Parse(s) if err != nil { - p.logger.Warn("failed to derive transport proxy from forward_proxy_url") + p.logger.Warn("failed to derive transport proxy from network_proxy URL") pUrl = nil } else if pUrl.Host == "" || strings.Split("", pUrl.Host)[0] == ":" { // url.Parse does not return an error on these values: @@ -86,7 +87,7 @@ func (p ProxyFromURL) ProxyFunc() func(*http.Request) (*url.URL, error) { // - pUrl.Host == "" // // Super edge cases, but humans are human. - err = errors.New("supplied forward_proxy_url is missing a host value") + err = errors.New("supplied network_proxy URL is missing a host value") pUrl = nil } else { p.logger.Debug("setting transport proxy url", zap.String("url", s)) @@ -108,6 +109,7 @@ func (p *ProxyFromURL) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { return nil } +// The "none" proxy source module disables the use of network proxy. type ProxyFromNone struct{} func (p ProxyFromNone) CaddyModule() caddy.ModuleInfo {