[go] net/http/httputil: add ReverseProxy.Rewriter
297 views
Skip to first unread message
Damien Neil (Gerrit)
May 18, 2022, 7:23:50 PM5/18/22
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Damien Neil has uploaded this change for review.
net/http/httputil: add ReverseProxy.Rewriter
Change-Id: Ib84e2fdd1d52c610e3887af66f517d4a74e594d0
---
M src/net/http/httputil/reverseproxy.go
1 file changed, 155 insertions(+), 53 deletions(-)
diff --git a/src/net/http/httputil/reverseproxy.go b/src/net/http/httputil/reverseproxy.go
index b5d3ce7..c4ba49d 100644
--- a/src/net/http/httputil/reverseproxy.go
+++ b/src/net/http/httputil/reverseproxy.go
@@ -8,6 +8,7 @@
import (
"context"
+ "errors"
"fmt"
"io"
"log"
@@ -24,29 +25,104 @@
"golang.org/x/net/http/httpguts"
)
+// A Rewriter contains a request to be rewritten by a ReverseProxy.
+type Rewriter struct {
+ // InReq is the request received by the proxy.
+ // The Rewrite function must not modify InReq.
+ InReq *http.Request
+
+ // OutReq is the request which will be sent by the proxy.
+ // The Rewrite function may modify this request.
+ // Hop-by-hop headers are removed from this request
+ // before Rewrite is called.
+ OutReq *http.Request
+}
+
+// SetURL routes the outbound request to the scheme, host, and base path
+// provided in target. If the target's path is "/base" and the incoming
+// request was for "/dir", the target request will be for "/base/dir".
+// SetURL does not rewrite the Host header.
+func (r *Rewriter) SetURL(target *url.URL) {
+ rewriteRequestURL(r.OutReq, target)
+}
+
+// SetXForwarded sets the X-Forwarded-For, X-Forwarded-Host, and
+// X-Forwarded-Proto headers of the outbound request.
+//
+// - The X-Forwarded-For header is set to the client IP address.
+// - The X-Forwarded-Host header is set to the host name requested
+//
+// by the client.
+//
+// - The X-Forwarded-Proto header is set to "http" or "https", depending
+//
+// on whether the inbound request was made on a TLS-enabled connection.
+func (r *Rewriter) SetXForwarded() {
+ clientIP, _, err := net.SplitHostPort(r.InReq.RemoteAddr)
+ if err == nil {
+ r.OutReq.Header.Set("X-Forwarded-For", clientIP)
+ } else {
+ r.OutReq.Header.Del("X-Forwarded-For")
+ }
+ r.setXForwardedHostProto()
+}
+
+// SetXForwarded sets the X-Forwarded-For, X-Forwarded-Host, and
+// X-Forwarded-Proto headers of the outbound request. It behaves
+// like SetXForwarded, but appends the client IP address to the
+// X-Forwarded-For header in the inbound request (if present).
+func (r *Rewriter) AppendXForwarded() {
+ clientIP, _, err := net.SplitHostPort(r.InReq.RemoteAddr)
+ if err != nil {
+ return
+ }
+ prior := r.OutReq.Header["X-Forwarded-For"]
+ if len(prior) > 0 {
+ clientIP = strings.Join(prior, ", ") + ", " + clientIP
+ }
+ r.OutReq.Header.Set("X-Forwarded-For", clientIP)
+}
+
+func (r *Rewriter) setXForwardedHostProto() {
+ r.OutReq.Header.Set("X-Forwarded-Host", r.InReq.Host)
+ if r.InReq.TLS == nil {
+ r.OutReq.Header.Set("X-Forwarded-Proto", "http")
+ } else {
+ r.OutReq.Header.Set("X-Forwarded-Proto", "https")
+ }
+}
+
// ReverseProxy is an HTTP Handler that takes an incoming request and
// sends it to another server, proxying the response back to the
// client.
-//
-// ReverseProxy by default sets the client IP as the value of the
-// X-Forwarded-For header.
-//
-// If an X-Forwarded-For header already exists, the client IP is
-// appended to the existing values. As a special case, if the header
-// exists in the Request.Header map but has a nil value (such as when
-// set by the Director func), the X-Forwarded-For header is
-// not modified.
-//
-// To prevent IP spoofing, be sure to delete any pre-existing
-// X-Forwarded-For header coming from the client or
-// an untrusted proxy.
type ReverseProxy struct {
- // Director must be a function which modifies
+ // Rewrite must be a function which modifies
// the request into a new request to be sent
// using Transport. Its response is then copied
// back to the original client unmodified.
- // Director must not access the provided Request
- // after returning.
+ // Rewrite must not access the provided Rewriter
+ // or its contents after returning.
+ //
+ // The X-Forwarded-For header is not automatically set when
+ // Rewrite is used. Use the Rewriter.SetXForwarded method
+ // to set it.
+ //
+ // At most one of Rewrite or Director may be set.
+ Rewrite func(*Rewriter)
+
+ // Director is a function which modifies the
+ // request into instead.
+ //
+ // The X-Forwarded-For header is set to the client IP after
+ // Director returns. If an X-Forwarded-For header already
+ // exists, the client IP is appended to the existing values.
+ // As a special case, if the header exists in the Request.Header
+ // map but has a nil value, the X-Forwarded-For header is not
+ // modified.
+ //
+ // To prevent IP spoofing, be sure to delete any pre-existing
+ // X-Forwarded-For header coming from the client or
+ // an untrusted proxy.
Director func(*http.Request)
// The transport used to perform proxy requests.
@@ -140,18 +216,10 @@
// the target request will be for /base/dir.
// NewSingleHostReverseProxy does not rewrite the Host header.
// To rewrite Host headers, use ReverseProxy directly with a custom
-// Director policy.
+// Rewrite policy.
func NewSingleHostReverseProxy(target *url.URL) *ReverseProxy {
- targetQuery := target.RawQuery
director := func(req *http.Request) {
- req.URL.Scheme = target.Scheme
- req.URL.Host = target.Host
- req.URL.Path, req.URL.RawPath = joinURLPath(target, req.URL)
- if targetQuery == "" || req.URL.RawQuery == "" {
- req.URL.RawQuery = targetQuery + req.URL.RawQuery
- } else {
- req.URL.RawQuery = targetQuery + "&" + req.URL.RawQuery
- }
+ rewriteRequestURL(req, target)
if _, ok := req.Header["User-Agent"]; !ok {
// explicitly disable User-Agent so it's not set to default value
req.Header.Set("User-Agent", "")
@@ -160,6 +228,18 @@
return &ReverseProxy{Director: director}
}
+func rewriteRequestURL(req *http.Request, target *url.URL) {
+ targetQuery := target.RawQuery
+ req.URL.Scheme = target.Scheme
+ req.URL.Host = target.Host
+ req.URL.Path, req.URL.RawPath = joinURLPath(target, req.URL)
+ if targetQuery == "" || req.URL.RawQuery == "" {
+ req.URL.RawQuery = targetQuery + req.URL.RawQuery
+ } else {
+ req.URL.RawQuery = targetQuery + "&" + req.URL.RawQuery
+ }
+}
+
func copyHeader(dst, src http.Header) {
for k, vv := range src {
for _, v := range vv {
@@ -221,13 +301,13 @@
if ctx.Done() != nil {
// CloseNotifier predates context.Context, and has been
// entirely superseded by it. If the request contains
- // a Context that carries a cancellation signal, don't
+ // a Context that carries a cancelation signal, don't
// bother spinning up a goroutine to watch the CloseNotify
// channel (if any).
//
// If the request Context has a nil Done channel (which
// means it is either context.Background, or a custom
- // Context implementation with no cancellation signal),
+ // Context implementation with no cancelation signal),
// then consult the CloseNotifier if available.
} else if cn, ok := rw.(http.CloseNotifier); ok {
var cancel context.CancelFunc
@@ -260,7 +340,14 @@
outreq.Header = make(http.Header) // Issue 33142: historical behavior was to always allocate
}
- p.Director(outreq)
+ if p.Director != nil && p.Rewrite != nil {
+ p.getErrorHandler()(rw, req, errors.New("ReverseProxy has Director and Rewrite set at the same time"))
+ return
+ }
+
+ if p.Director != nil {
+ p.Director(outreq)
+ }
outreq.Close = false
reqUpType := upgradeType(outreq.Header)
@@ -268,20 +355,13 @@
p.getErrorHandler()(rw, req, fmt.Errorf("client tried to switch to invalid protocol %q", reqUpType))
return
}
- removeConnectionHeaders(outreq.Header)
-
- // Remove hop-by-hop headers to the backend. Especially
- // important is "Connection" because we want a persistent
- // connection, regardless of what the client sent to us.
- for _, h := range hopHeaders {
- outreq.Header.Del(h)
- }
+ removeHopByHopHeaders(outreq.Header)
// Issue 21096: tell backend applications that care about trailer support
// that we support trailers. (We do, but we don't go out of our way to
// advertise that unless the incoming client request thought it was worth
// mentioning.) Note that we look at req.Header, not outreq.Header, since
- // the latter has passed through removeConnectionHeaders.
+ // the latter has passed through removeHopByHopHeaders.
if httpguts.HeaderValuesContainsToken(req.Header["Te"], "trailers") {
outreq.Header.Set("Te", "trailers")
}
@@ -293,17 +373,24 @@
outreq.Header.Set("Upgrade", reqUpType)
}
- if clientIP, _, err := net.SplitHostPort(req.RemoteAddr); err == nil {
- // If we aren't the first proxy retain prior
- // X-Forwarded-For information as a comma+space
- // separated list and fold multiple headers into one.
- prior, ok := outreq.Header["X-Forwarded-For"]
- omit := ok && prior == nil // Issue 38079: nil now means don't populate the header
- if len(prior) > 0 {
- clientIP = strings.Join(prior, ", ") + ", " + clientIP
- }
- if !omit {
- outreq.Header.Set("X-Forwarded-For", clientIP)
+ if p.Rewrite != nil {
+ p.Rewrite(&Rewriter{
+ InReq: req,
+ OutReq: outreq,
+ })
+ } else {
+ if clientIP, _, err := net.SplitHostPort(req.RemoteAddr); err == nil {
+ // If we aren't the first proxy retain prior
+ // X-Forwarded-For information as a comma+space
+ // separated list and fold multiple headers into one.
+ prior, ok := outreq.Header["X-Forwarded-For"]
+ omit := ok && prior == nil // Issue 38079: nil now means don't populate the header
+ if len(prior) > 0 {
+ clientIP = strings.Join(prior, ", ") + ", " + clientIP
+ }
+ if !omit {
+ outreq.Header.Set("X-Forwarded-For", clientIP)
+ }
}
}
@@ -322,7 +409,7 @@
return
}
- removeConnectionHeaders(res.Header)
+ removeHopByHopHeaders(res.Header)
for _, h := range hopHeaders {
res.Header.Del(h)
@@ -405,9 +492,9 @@
return false
}
-// removeConnectionHeaders removes hop-by-hop headers listed in the "Connection" header of h.
-// See RFC 7230, section 6.1
-func removeConnectionHeaders(h http.Header) {
+// removeHopByHopHeaders removes hop-by-hop headers.
+func removeHopByHopHeaders(h http.Header) {
+ // RFC 7230, section 6.1: Remove headers listed in the "Connection" header.
for _, f := range h["Connection"] {
for _, sf := range strings.Split(f, ",") {
if sf = textproto.TrimString(sf); sf != "" {
@@ -415,6 +502,12 @@
}
}
}
+ // RFC 2616, section 13.5.1: Remove a set of known hop-by-hop headers.
+ // This behavior is superseded by the RFC 7230 Connection header, but
+ // preserve it for backwards compatibility.
+ for _, f := range hopHeaders {
+ h.Del(f)
+ }
}
// flushInterval returns the p.FlushInterval value, conditionally
To view, visit change 407214. To unsubscribe, or for help writing mail filters, visit settings.
Damien Neil (Gerrit)
May 19, 2022, 3:10:02 PM5/19/22
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Damien Neil uploaded patch set #2 to this change.
net/http/httputil: add ReverseProxy.Rewriter
Change-Id: Ib84e2fdd1d52c610e3887af66f517d4a74e594d0
---
M src/net/http/httputil/reverseproxy.go
1 file changed, 152 insertions(+), 53 deletions(-)
To view, visit change 407214. To unsubscribe, or for help writing mail filters, visit settings.
Damien Neil (Gerrit)
May 19, 2022, 3:11:35 PM5/19/22
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Damien Neil uploaded patch set #3 to this change.
net/http/httputil: add ReverseProxy.Rewriter
Change-Id: Ib84e2fdd1d52c610e3887af66f517d4a74e594d0
---
M src/net/http/httputil/reverseproxy.go
1 file changed, 150 insertions(+), 55 deletions(-)
To view, visit change 407214. To unsubscribe, or for help writing mail filters, visit settings.
Damien Neil (Gerrit)
May 19, 2022, 3:15:28 PM5/19/22
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Damien Neil uploaded patch set #4 to this change.
net/http/httputil: add ReverseProxy.Rewriter
Change-Id: Ib84e2fdd1d52c610e3887af66f517d4a74e594d0
---
M src/net/http/httputil/reverseproxy.go
1 file changed, 156 insertions(+), 59 deletions(-)
To view, visit change 407214. To unsubscribe, or for help writing mail filters, visit settings.
Damien Neil (Gerrit)
Jun 10, 2022, 7:29:32 PM6/10/22
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Damien Neil uploaded patch set #5 to this change.
net/http/httputil: add ReverseProxy.Rewrite
Add a new Rewrite hook to ReverseProxy, superseding the Director hook.
Director does not distinguish between the inbound and outbound request,
which makes it possible for headers added by Director to be inadvertently
removed before forwarding if they are listed in the inbound request's
Connection header. Rewrite accepts a value containing the inbound
and outbound requests, with hop-by-hop headers already removed from
the outbound request, avoiding this problem.
ReverseProxy's appends the client IP to the inbound X-Forwarded-For
header by default. Users must manually delete untrusted X-Forwarded-For
values. When used with a Rewrite hook, ReverseProxy now strips
X-Forwarded-* headers by default.
NewSingleHostReverseProxy creates a proxy that does not rewrite the
Host header of inbound requests. Changing this behavior is
cumbersome, as it requires wrapping the Director function created
by NewSingleHostReverseProxy. The Rewrite hook's ProxyRequest
parameter provides a SetURL method that provides equivalent
functionality to NewSingleHostReverseProxy, rewrites the Host
header by default, and can be more easily extended with additional
customizations.
Fixes #28168.
Fixes #50580.
Fixes #53002.
Change-Id: Ib84e2fdd1d52c610e3887af66f517d4a74e594d0
---
M src/net/http/httputil/reverseproxy.go
M src/net/http/httputil/reverseproxy_test.go
2 files changed, 291 insertions(+), 58 deletions(-)
To view, visit change 407214. To unsubscribe, or for help writing mail filters, visit settings.
Damien Neil (Gerrit)
Jul 29, 2022, 4:35:51 PM7/29/22
to goph...@pubsubhelper.golang.org, Brad Fitzpatrick, Gopher Robot, Russ Cox, golang-co...@googlegroups.com
Attention is currently required from: Brad Fitzpatrick.
1 comment:
File src/net/http/httputil/reverseproxy.go:
Patch Set #5, Line 399:
// The Rewrite func needs tofinish sentence fragment
To view, visit change 407214. To unsubscribe, or for help writing mail filters, visit settings.
Damien Neil (Gerrit)
Aug 2, 2022, 1:16:00 PM8/2/22
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention is currently required from: Brad Fitzpatrick, Damien Neil.
Damien Neil uploaded patch set #6 to this change.
The following approvals got outdated and were removed: Run-TryBot+1 by Damien Neil, TryBot-Result-1 by Gopher Robot
A api/next/53002.txt
M src/net/http/httputil/example_test.go
M src/net/http/httputil/reverseproxy.go
M src/net/http/httputil/reverseproxy_test.go
4 files changed, 304 insertions(+), 59 deletions(-)
To view, visit change 407214. To unsubscribe, or for help writing mail filters, visit settings.
Damien Neil (Gerrit)
Aug 5, 2022, 12:08:11 PM8/5/22
to goph...@pubsubhelper.golang.org, Brad Fitzpatrick, Gopher Robot, Russ Cox, golang-co...@googlegroups.com
Attention is currently required from: Brad Fitzpatrick.
Patch set 6:Run-TryBot +1
Damien Neil (Gerrit)
Aug 5, 2022, 12:09:27 PM8/5/22
to goph...@pubsubhelper.golang.org, Brad Fitzpatrick, Gopher Robot, Russ Cox, golang-co...@googlegroups.com
Attention is currently required from: Brad Fitzpatrick.
1 comment:
File src/net/http/httputil/reverseproxy.go:
Patch Set #5, Line 399:
// The Rewrite func needs tofinish sentence fragment
Done
To view, visit change 407214. To unsubscribe, or for help writing mail filters, visit settings.
Brad Fitzpatrick (Gerrit)
Aug 5, 2022, 12:10:24 PM8/5/22
to Damien Neil, goph...@pubsubhelper.golang.org, Brad Fitzpatrick, Gopher Robot, Russ Cox, golang-co...@googlegroups.com
Attention is currently required from: Damien Neil.
1 comment:
File src/net/http/httputil/reverseproxy.go:
Patch Set #6, Line 32:
InReq *http.RequestIn and Out instead? "Req" is weirdly short for a field name.
To view, visit change 407214. To unsubscribe, or for help writing mail filters, visit settings.
Damien Neil (Gerrit)
Aug 5, 2022, 12:13:26 PM8/5/22
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention is currently required from: Damien Neil.
Damien Neil uploaded patch set #7 to this change.
The following approvals got outdated and were removed: Run-TryBot+1 by Damien Neil
net/http/httputil: add ReverseProxy.Rewrite
To view, visit change 407214. To unsubscribe, or for help writing mail filters, visit settings.
Damien Neil (Gerrit)
Aug 5, 2022, 12:15:38 PM8/5/22
to goph...@pubsubhelper.golang.org, Brad Fitzpatrick, Gopher Robot, Russ Cox, golang-co...@googlegroups.com
Patch set 7:Run-TryBot +1
Brad Fitzpatrick (Gerrit)
Aug 5, 2022, 12:17:52 PM8/5/22
to Damien Neil, goph...@pubsubhelper.golang.org, Brad Fitzpatrick, Gopher Robot, Russ Cox, golang-co...@googlegroups.com
1 comment:
File src/net/http/httputil/reverseproxy.go:
save ProxyRequest and then add:
outreq = pr.Out
So people can change the overall request? And document?
To view, visit change 407214. To unsubscribe, or for help writing mail filters, visit settings.
Brad Fitzpatrick (Gerrit)
Aug 5, 2022, 12:20:05 PM8/5/22
to Damien Neil, goph...@pubsubhelper.golang.org, Brad Fitzpatrick, Gopher Robot, Russ Cox, golang-co...@googlegroups.com
Attention is currently required from: Damien Neil.
1 comment:
File src/net/http/httputil/reverseproxy.go:
Patch Set #7, Line 110:
// the ProxyRequest.SetXForwarded methotypo/truncated
To view, visit change 407214. To unsubscribe, or for help writing mail filters, visit settings.
Brad Fitzpatrick (Gerrit)
Aug 5, 2022, 12:22:55 PM8/5/22
to Damien Neil, goph...@pubsubhelper.golang.org, Brad Fitzpatrick, Gopher Robot, Russ Cox, golang-co...@googlegroups.com
File src/net/http/httputil/reverseproxy.go:
r.Out.Host = r.In.Host // if desired
To view, visit change 407214. To unsubscribe, or for help writing mail filters, visit settings.
Damien Neil (Gerrit)
Aug 5, 2022, 12:27:30 PM8/5/22
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention is currently required from: Damien Neil.
Damien Neil uploaded patch set #8 to this change.
The following approvals got outdated and were removed: Run-TryBot+1 by Damien Neil, TryBot-Result-1 by Gopher Robot
net/http/httputil: add ReverseProxy.Rewrite
4 files changed, 330 insertions(+), 58 deletions(-)
To view, visit change 407214. To unsubscribe, or for help writing mail filters, visit settings.
Brad Fitzpatrick (Gerrit)
Aug 5, 2022, 12:43:40 PM8/5/22
to Damien Neil, goph...@pubsubhelper.golang.org, Brad Fitzpatrick, Gopher Robot, Russ Cox, golang-co...@googlegroups.com
Attention is currently required from: Damien Neil.
Patch set 8:Code-Review +2
Damien Neil (Gerrit)
Aug 15, 2022, 6:10:44 PM8/15/22
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention is currently required from: Damien Neil.
Damien Neil uploaded patch set #9 to this change.
net/http/httputil: add ReverseProxy.Rewrite
4 files changed, 340 insertions(+), 67 deletions(-)
To view, visit change 407214. To unsubscribe, or for help writing mail filters, visit settings.
Damien Neil (Gerrit)
Aug 15, 2022, 6:13:50 PM8/15/22
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention is currently required from: Damien Neil.
Damien Neil uploaded patch set #10 to this change.
4 files changed, 341 insertions(+), 67 deletions(-)
To view, visit change 407214. To unsubscribe, or for help writing mail filters, visit settings.
Damien Neil (Gerrit)
Aug 15, 2022, 6:14:42 PM8/15/22
to goph...@pubsubhelper.golang.org, Brad Fitzpatrick, Gopher Robot, Russ Cox, golang-co...@googlegroups.com
4 comments:
File src/net/http/httputil/reverseproxy.go:
Patch Set #6, Line 32:
InReq *http.RequestIn and Out instead? "Req" is weirdly short for a field name.
Done
File src/net/http/httputil/reverseproxy.go:
Patch Set #7, Line 110:
// the ProxyRequest.SetXForwarded methotypo/truncated
Done
r.Out.Host = r.In. […]
Done
save ProxyRequest and then add: […]
Done
To view, visit change 407214. To unsubscribe, or for help writing mail filters, visit settings.
Damien Neil (Gerrit)
Aug 15, 2022, 6:14:59 PM8/15/22
to goph...@pubsubhelper.golang.org, Brad Fitzpatrick, Gopher Robot, Russ Cox, golang-co...@googlegroups.com
Patch set 10:Run-TryBot +1
Damien Neil (Gerrit)
Aug 15, 2022, 6:46:33 PM8/15/22
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention is currently required from: Damien Neil.
Damien Neil uploaded patch set #11 to this change.
The following approvals got outdated and were removed: Run-TryBot+1 by Damien Neil, TryBot-Result-1 by Gopher Robot
net/http/httputil: add ReverseProxy.Rewrite
To view, visit change 407214. To unsubscribe, or for help writing mail filters, visit settings.
Damien Neil (Gerrit)
Aug 15, 2022, 6:57:20 PM8/15/22
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention is currently required from: Damien Neil.
Damien Neil uploaded patch set #12 to this change.
net/http/httputil: add ReverseProxy.Rewrite
Damien Neil (Gerrit)
Aug 15, 2022, 6:57:32 PM8/15/22
to goph...@pubsubhelper.golang.org, Gopher Robot, Brad Fitzpatrick, Russ Cox, golang-co...@googlegroups.com
Patch set 12:Run-TryBot +1
Damien Neil (Gerrit)
Aug 15, 2022, 8:47:25 PM8/15/22
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention is currently required from: Damien Neil.
Damien Neil uploaded patch set #13 to this change.
The following approvals got outdated and were removed: Run-TryBot+1 by Damien Neil, TryBot-Result-1 by Gopher Robot
net/http/httputil: add ReverseProxy.Rewrite
To view, visit change 407214. To unsubscribe, or for help writing mail filters, visit settings.
Damien Neil (Gerrit)
Aug 15, 2022, 8:48:00 PM8/15/22
to goph...@pubsubhelper.golang.org, Gopher Robot, Brad Fitzpatrick, Russ Cox, golang-co...@googlegroups.com
Patch set 13:Run-TryBot +1
Damien Neil (Gerrit)
Aug 15, 2022, 9:17:35 PM8/15/22
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention is currently required from: Damien Neil.
Damien Neil uploaded patch set #14 to this change.
To view, visit change 407214. To unsubscribe, or for help writing mail filters, visit settings.
Damien Neil (Gerrit)
Aug 15, 2022, 10:47:30 PM8/15/22
to goph...@pubsubhelper.golang.org, Gopher Robot, Brad Fitzpatrick, Russ Cox, golang-co...@googlegroups.com
Patch set 14:Run-TryBot +1
Roland Shoemaker (Gerrit)
Aug 16, 2022, 3:25:08 PM8/16/22
to Damien Neil, goph...@pubsubhelper.golang.org, Gopher Robot, Brad Fitzpatrick, Russ Cox, golang-co...@googlegroups.com
Attention is currently required from: Damien Neil.
Patch set 14:Code-Review +1
Damien Neil (Gerrit)
Aug 16, 2022, 4:01:47 PM8/16/22
to goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Roland Shoemaker, Gopher Robot, Brad Fitzpatrick, Russ Cox, golang-co...@googlegroups.com
Damien Neil submitted this change.
8 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: api/next/53002.txt
Insertions: 6, Deletions: 6.
The diff is too large to show. Please review the diff.
```
```
The name of the file: src/net/http/httputil/reverseproxy.go
Insertions: 28, Deletions: 10.
The diff is too large to show. Please review the diff.
```
Approvals:
Damien Neil: Run TryBots
Roland Shoemaker: Looks good to me, but someone else must approve
Gopher Robot: TryBots succeeded
Brad Fitzpatrick: Looks good to me, approved
net/http/httputil: add ReverseProxy.Rewrite
Add a new Rewrite hook to ReverseProxy, superseding the Director hook.
Director does not distinguish between the inbound and outbound request,
which makes it possible for headers added by Director to be inadvertently
removed before forwarding if they are listed in the inbound request's
Connection header. Rewrite accepts a value containing the inbound
and outbound requests, with hop-by-hop headers already removed from
the outbound request, avoiding this problem.
ReverseProxy's appends the client IP to the inbound X-Forwarded-For
header by default. Users must manually delete untrusted X-Forwarded-For
values. When used with a Rewrite hook, ReverseProxy now strips
X-Forwarded-* headers by default.
NewSingleHostReverseProxy creates a proxy that does not rewrite the
Host header of inbound requests. Changing this behavior is
cumbersome, as it requires wrapping the Director function created
by NewSingleHostReverseProxy. The Rewrite hook's ProxyRequest
parameter provides a SetURL method that provides equivalent
functionality to NewSingleHostReverseProxy, rewrites the Host
header by default, and can be more easily extended with additional
customizations.
Fixes #28168.
Fixes #50580.
Fixes #53002.
Change-Id: Ib84e2fdd1d52c610e3887af66f517d4a74e594d0
Reviewed-on: https://go-review.googlesource.com/c/go/+/407214
Reviewed-by: Brad Fitzpatrick <brad...@golang.org>
TryBot-Result: Gopher Robot <go...@golang.org>
Reviewed-by: Roland Shoemaker <rol...@golang.org>
Run-TryBot: Damien Neil <dn...@google.com>
---
A api/next/53002.txt
M src/net/http/httputil/example_test.go
M src/net/http/httputil/reverseproxy.go
M src/net/http/httputil/reverseproxy_test.go
4 files changed, 346 insertions(+), 67 deletions(-)
diff --git a/api/next/53002.txt b/api/next/53002.txt
new file mode 100644
index 0000000..b078fee
--- /dev/null
+++ b/api/next/53002.txt
@@ -0,0 +1,6 @@
+pkg net/http/httputil, method (*ProxyRequest) SetURL(*url.URL) #53002
+pkg net/http/httputil, method (*ProxyRequest) SetXForwarded() #53002
+pkg net/http/httputil, type ProxyRequest struct #53002
+pkg net/http/httputil, type ProxyRequest struct, In *http.Request #53002
+pkg net/http/httputil, type ProxyRequest struct, Out *http.Request #53002
+pkg net/http/httputil, type ReverseProxy struct, Rewrite func(*ProxyRequest) #53002
diff --git a/src/net/http/httputil/example_test.go b/src/net/http/httputil/example_test.go
index b77a243..6c107f8 100644
--- a/src/net/http/httputil/example_test.go
+++ b/src/net/http/httputil/example_test.go
@@ -103,7 +103,12 @@
if err != nil {
log.Fatal(err)
}
- frontendProxy := httptest.NewServer(httputil.NewSingleHostReverseProxy(rpURL))
+ frontendProxy := httptest.NewServer(&httputil.ReverseProxy{
+ Rewrite: func(r *httputil.ProxyRequest) {
+ r.SetXForwarded()
+ r.SetURL(rpURL)
+ },
+ })
defer frontendProxy.Close()
resp, err := http.Get(frontendProxy.URL)
diff --git a/src/net/http/httputil/reverseproxy.go b/src/net/http/httputil/reverseproxy.go
index 0c52497..11711e6 100644
--- a/src/net/http/httputil/reverseproxy.go
+++ b/src/net/http/httputil/reverseproxy.go
@@ -8,6 +8,7 @@
import (
"context"
+ "errors"
"fmt"
"io"
"log"
@@ -24,33 +25,118 @@
"golang.org/x/net/http/httpguts"
)
+// A ProxyRequest contains a request to be rewritten by a ReverseProxy.
+type ProxyRequest struct {
+ // In is the request received by the proxy.
+ // The Rewrite function must not modify In.
+ In *http.Request
+
+ // Out is the request which will be sent by the proxy.
+ // The Rewrite function may modify or replace this request.
+ // Hop-by-hop headers are removed from this request
+ // before Rewrite is called.
+ Out *http.Request
+}
+
+// SetURL routes the outbound request to the scheme, host, and base path
+// provided in target. If the target's path is "/base" and the incoming
+// request was for "/dir", the target request will be for "/base/dir".
+//
+// SetURL rewrites the outbound Host header to match the target's host.
+// To preserve the inbound request's Host header (the default behavior
+// of NewSingleHostReverseProxy):
+//
+// rewriteFunc := func(r *httputil.ProxyRequest) {
+// r.SetURL(url)
+// r.Out.Host = r.In.Host
+// }
+func (r *ProxyRequest) SetURL(target *url.URL) {
+ rewriteRequestURL(r.Out, target)
+ r.Out.Host = ""
+}
+
+// SetXForwarded sets the X-Forwarded-For, X-Forwarded-Host, and
+// X-Forwarded-Proto headers of the outbound request.
+//
+// - The X-Forwarded-For header is set to the client IP address.
+// - The X-Forwarded-Host header is set to the host name requested
+// by the client.
+// - The X-Forwarded-Proto header is set to "http" or "https", depending
+// on whether the inbound request was made on a TLS-enabled connection.
+//
+// If the outbound request contains an existing X-Forwarded-For header,
+// SetXForwarded appends the client IP address to it. To append to the
+// inbound request's X-Forwarded-For header (the default behavior of
+// ReverseProxy when using a Director function), copy the header
+// from the inbound request before calling SetXForwarded:
+//
+// rewriteFunc := func(r *httputil.ProxyRequest) {
+// r.Out.Header["X-Forwarded-For"] = r.In.Header["X-Forwarded-For"]
+// r.SetXForwarded()
+// }
+func (r *ProxyRequest) SetXForwarded() {
+ clientIP, _, err := net.SplitHostPort(r.In.RemoteAddr)
+ if err == nil {
+ prior := r.Out.Header["X-Forwarded-For"]
+ if len(prior) > 0 {
+ clientIP = strings.Join(prior, ", ") + ", " + clientIP
+ }
+ r.Out.Header.Set("X-Forwarded-For", clientIP)
+ } else {
+ r.Out.Header.Del("X-Forwarded-For")
+ }
+ r.Out.Header.Set("X-Forwarded-Host", r.In.Host)
+ if r.In.TLS == nil {
+ r.Out.Header.Set("X-Forwarded-Proto", "http")
+ } else {
+ r.Out.Header.Set("X-Forwarded-Proto", "https")
+ }
+}
+
// ReverseProxy is an HTTP Handler that takes an incoming request and
// sends it to another server, proxying the response back to the
// client.
-//
-// ReverseProxy by default sets
-// - the X-Forwarded-For header to the client IP address;
-// - the X-Forwarded-Host header to the host of the original client
-// request; and
-// - the X-Forwarded-Proto header to "https" if the client request
-// was made on a TLS-enabled connection or "http" otherwise.
-//
-// If an X-Forwarded-For header already exists, the client IP is
-// appended to the existing values.
-//
-// If a header exists in the Request.Header map but has a nil value
-// (such as when set by the Director func), it is not modified.
-//
-// To prevent IP spoofing, be sure to delete any pre-existing
-// X-Forwarded-For header coming from the client or
-// an untrusted proxy.
type ReverseProxy struct {
- // Director must be a function which modifies
+ // Rewrite must be a function which modifies
+ // the request into a new request to be sent
+ // using Transport. Its response is then copied
+ // back to the original client unmodified.
+ // Rewrite must not access the provided ProxyRequest
+ // or its contents after returning.
+ //
+ // The Forwarded, X-Forwarded, X-Forwarded-Host,
+ // and X-Forwarded-Proto headers are removed from the
+ // outbound request before Rewrite is called. See also
+ // the ProxyRequest.SetXForwarded method.
+ //
+ // At most one of Rewrite or Director may be set.
+ Rewrite func(*ProxyRequest)
+
+ // Director is a function which modifies the
// the request into a new request to be sent
// using Transport. Its response is then copied
// back to the original client unmodified.
// Director must not access the provided Request
// after returning.
+ //
+ // By default, the X-Forwarded-For, X-Forwarded-Host, and
+ // X-Forwarded-Proto headers of the ourgoing request are
+ // set as by the ProxyRequest.SetXForwarded function.
+ //
+ // If an X-Forwarded-For header already exists, the client IP is
+ // appended to the existing values. To prevent IP spoofing, be
+ // sure to delete any pre-existing X-Forwarded-For header
+ // coming from the client or an untrusted proxy.
+ //
+ // If a header exists in the Request.Header map but has a nil value
+ // (such as when set by the Director func), it is not modified.
+ //
+ // Hop-by-hop headers are removed from the request after
+ // Director returns, which can remove headers added by
+ // Director. Use a Rewrite function instead to ensure
+ // modifications to the request are preserved.
+ //
+ // At most one of Rewrite or Director may be set.
Director func(*http.Request)
// The transport used to perform proxy requests.
@@ -142,24 +228,41 @@
// URLs to the scheme, host, and base path provided in target. If the
// target's path is "/base" and the incoming request was for "/dir",
// the target request will be for /base/dir.
+//
// NewSingleHostReverseProxy does not rewrite the Host header.
-// To rewrite Host headers, use ReverseProxy directly with a custom
-// Director policy.
+//
+// To customize the ReverseProxy behavior beyond what
+// NewSingleHostReverseProxy provides, use ReverseProxy directly
+// with a Rewrite function. The ProxyRequest SetURL method
+// may be used to route the outbound request. (Note that SetURL,
+// unlike NewSingleHostReverseProxy, rewrites the Host header
+// of the outbound request by default.)
+//
+// proxy := &ReverseProxy{
+// Rewrite: func(r *ProxyRequest) {
+// r.SetURL(target)
+// r.Out.Host = r.In.Host // if desired
+// }
+// }
func NewSingleHostReverseProxy(target *url.URL) *ReverseProxy {
- targetQuery := target.RawQuery
director := func(req *http.Request) {
- req.URL.Scheme = target.Scheme
- req.URL.Host = target.Host
- req.URL.Path, req.URL.RawPath = joinURLPath(target, req.URL)
- if targetQuery == "" || req.URL.RawQuery == "" {
- req.URL.RawQuery = targetQuery + req.URL.RawQuery
- } else {
- req.URL.RawQuery = targetQuery + "&" + req.URL.RawQuery
- }
+ rewriteRequestURL(req, target)
}
return &ReverseProxy{Director: director}
}
+func rewriteRequestURL(req *http.Request, target *url.URL) {
+ targetQuery := target.RawQuery
+ req.URL.Scheme = target.Scheme
+ req.URL.Host = target.Host
+ req.URL.Path, req.URL.RawPath = joinURLPath(target, req.URL)
+ if targetQuery == "" || req.URL.RawQuery == "" {
+ req.URL.RawQuery = targetQuery + req.URL.RawQuery
+ } else {
+ req.URL.RawQuery = targetQuery + "&" + req.URL.RawQuery
+ }
+}
+
func copyHeader(dst, src http.Header) {
for k, vv := range src {
for _, v := range vv {
@@ -260,7 +363,14 @@
outreq.Header = make(http.Header) // Issue 33142: historical behavior was to always allocate
}
- p.Director(outreq)
+ if (p.Director != nil) == (p.Rewrite != nil) {
+ p.getErrorHandler()(rw, req, errors.New("ReverseProxy must have exactly one of Director or Rewrite set"))
+ return
+ }
+
+ if p.Director != nil {
+ p.Director(outreq)
+ }
outreq.Close = false
reqUpType := upgradeType(outreq.Header)
@@ -268,20 +378,13 @@
p.getErrorHandler()(rw, req, fmt.Errorf("client tried to switch to invalid protocol %q", reqUpType))
return
}
- removeConnectionHeaders(outreq.Header)
-
- // Remove hop-by-hop headers to the backend. Especially
- // important is "Connection" because we want a persistent
- // connection, regardless of what the client sent to us.
- for _, h := range hopHeaders {
- outreq.Header.Del(h)
- }
+ removeHopByHopHeaders(outreq.Header)
// Issue 21096: tell backend applications that care about trailer support
// that we support trailers. (We do, but we don't go out of our way to
// advertise that unless the incoming client request thought it was worth
// mentioning.) Note that we look at req.Header, not outreq.Header, since
- // the latter has passed through removeConnectionHeaders.
+ // the latter has passed through removeHopByHopHeaders.
if httpguts.HeaderValuesContainsToken(req.Header["Te"], "trailers") {
outreq.Header.Set("Te", "trailers")
}
@@ -293,28 +396,51 @@
outreq.Header.Set("Upgrade", reqUpType)
}
- if clientIP, _, err := net.SplitHostPort(req.RemoteAddr); err == nil {
- // If we aren't the first proxy retain prior
- // X-Forwarded-For information as a comma+space
- // separated list and fold multiple headers into one.
- prior, ok := outreq.Header["X-Forwarded-For"]
- omit := ok && prior == nil // Issue 38079: nil now means don't populate the header
- if len(prior) > 0 {
- clientIP = strings.Join(prior, ", ") + ", " + clientIP
+ if p.Rewrite != nil {
+ // Strip client-provided forwarding headers.
+ // The Rewrite func may use SetXForwarded to set new values
+ // for these or copy the previous values from the inbound request.
+ outreq.Header.Del("Forwarded")
+ outreq.Header.Del("X-Forwarded-For")
+ outreq.Header.Del("X-Forwarded-Host")
+ outreq.Header.Del("X-Forwarded-Proto")
+
+ pr := &ProxyRequest{
+ In: req,
+ Out: outreq,
}
- if !omit {
- outreq.Header.Set("X-Forwarded-For", clientIP)
+ p.Rewrite(pr)
+ outreq = pr.Out
+ } else {
+ if clientIP, _, err := net.SplitHostPort(req.RemoteAddr); err == nil {
+ // If we aren't the first proxy retain prior
+ // X-Forwarded-For information as a comma+space
+ // separated list and fold multiple headers into one.
+ prior, ok := outreq.Header["X-Forwarded-For"]
+ omit := ok && prior == nil // Issue 38079: nil now means don't populate the header
+ if len(prior) > 0 {
+ clientIP = strings.Join(prior, ", ") + ", " + clientIP
+ }
+ if !omit {
+ outreq.Header.Set("X-Forwarded-For", clientIP)
+ }
+ }
+ if prior, ok := outreq.Header["X-Forwarded-Host"]; !(ok && prior == nil) {
+ outreq.Header.Set("X-Forwarded-Host", req.Host)
+ }
+ if prior, ok := outreq.Header["X-Forwarded-Proto"]; !(ok && prior == nil) {
+ if req.TLS == nil {
+ outreq.Header.Set("X-Forwarded-Proto", "http")
+ } else {
+ outreq.Header.Set("X-Forwarded-Proto", "https")
+ }
}
}
- if prior, ok := outreq.Header["X-Forwarded-Host"]; !(ok && prior == nil) {
- outreq.Header.Set("X-Forwarded-Host", req.Host)
- }
- if prior, ok := outreq.Header["X-Forwarded-Proto"]; !(ok && prior == nil) {
- if req.TLS == nil {
- outreq.Header.Set("X-Forwarded-Proto", "http")
- } else {
- outreq.Header.Set("X-Forwarded-Proto", "https")
- }
+
+ if _, ok := outreq.Header["User-Agent"]; !ok {
+ // If the outbound request doesn't have a User-Agent header set,
+ // don't send the default Go HTTP client User-Agent.
+ outreq.Header.Set("User-Agent", "")
}
if _, ok := outreq.Header["User-Agent"]; !ok {
@@ -338,11 +464,7 @@
return
}
- removeConnectionHeaders(res.Header)
-
- for _, h := range hopHeaders {
- res.Header.Del(h)
- }
+ removeHopByHopHeaders(res.Header)
if !p.modifyResponse(rw, res, outreq) {
return
@@ -421,9 +543,9 @@
return false
}
-// removeConnectionHeaders removes hop-by-hop headers listed in the "Connection" header of h.
-// See RFC 7230, section 6.1
-func removeConnectionHeaders(h http.Header) {
+// removeHopByHopHeaders removes hop-by-hop headers.
+func removeHopByHopHeaders(h http.Header) {
+ // RFC 7230, section 6.1: Remove headers listed in the "Connection" header.
for _, f := range h["Connection"] {
for _, sf := range strings.Split(f, ",") {
if sf = textproto.TrimString(sf); sf != "" {
@@ -431,6 +553,12 @@
}
}
}
+ // RFC 2616, section 13.5.1: Remove a set of known hop-by-hop headers.
+ // This behavior is superseded by the RFC 7230 Connection header, but
+ // preserve it for backwards compatibility.
+ for _, f := range hopHeaders {
+ h.Del(f)
+ }
}
// flushInterval returns the p.FlushInterval value, conditionally
diff --git a/src/net/http/httputil/reverseproxy_test.go b/src/net/http/httputil/reverseproxy_test.go
index 3090e37..f8157e9 100644
--- a/src/net/http/httputil/reverseproxy_test.go
+++ b/src/net/http/httputil/reverseproxy_test.go
@@ -409,6 +409,46 @@
res.Body.Close()
}
+func TestReverseProxyRewriteStripsForwarded(t *testing.T) {
+ headers := []string{
+ "Forwarded",
+ "X-Forwarded-For",
+ "X-Forwarded-Host",
+ "X-Forwarded-Proto",
+ }
+ backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ for _, h := range headers {
+ if v := r.Header.Get(h); v != "" {
+ t.Errorf("got %v header: %q", h, v)
+ }
+ }
+ }))
+ defer backend.Close()
+ backendURL, err := url.Parse(backend.URL)
+ if err != nil {
+ t.Fatal(err)
+ }
+ proxyHandler := &ReverseProxy{
+ Rewrite: func(r *ProxyRequest) {
+ r.SetURL(backendURL)
+ },
+ }
+ frontend := httptest.NewServer(proxyHandler)
+ defer frontend.Close()
+
+ getReq, _ := http.NewRequest("GET", frontend.URL, nil)
+ getReq.Host = "some-name"
+ getReq.Close = true
+ for _, h := range headers {
+ getReq.Header.Set(h, "x")
+ }
+ res, err := frontend.Client().Do(getReq)
+ if err != nil {
+ t.Fatalf("Get: %v", err)
+ }
+ res.Body.Close()
+}
+
var proxyQueryTests = []struct {
baseSuffix string // suffix to add to backend URL
reqSuffix string // suffix to add to frontend's request URL
@@ -1523,6 +1563,40 @@
}
+func TestSetURL(t *testing.T) {
+ backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ w.Write([]byte(r.Host))
+ }))
+ defer backend.Close()
+ backendURL, err := url.Parse(backend.URL)
+ if err != nil {
+ t.Fatal(err)
+ }
+ proxyHandler := &ReverseProxy{
+ Rewrite: func(r *ProxyRequest) {
+ r.SetURL(backendURL)
+ },
+ }
+ frontend := httptest.NewServer(proxyHandler)
+ defer frontend.Close()
+ frontendClient := frontend.Client()
+
+ res, err := frontendClient.Get(frontend.URL)
+ if err != nil {
+ t.Fatalf("Get: %v", err)
+ }
+ defer res.Body.Close()
+
+ body, err := io.ReadAll(res.Body)
+ if err != nil {
+ t.Fatalf("Reading body: %v", err)
+ }
+
+ if got, want := string(body), backendURL.Host; got != want {
+ t.Errorf("backend got Host %q, want %q", got, want)
+ }
+}
+
func TestSingleJoinSlash(t *testing.T) {
tests := []struct {
slasha string
@@ -1572,3 +1646,28 @@
}
}
}
+
+func TestReverseProxyRewriteReplacesOut(t *testing.T) {
+ const content = "response_content"
+ backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+ w.Write([]byte(content))
+ }))
+ defer backend.Close()
+ proxyHandler := &ReverseProxy{
+ Rewrite: func(r *ProxyRequest) {
+ r.Out, _ = http.NewRequest("GET", backend.URL, nil)
+ },
+ }
+ frontend := httptest.NewServer(proxyHandler)
+ defer frontend.Close()
+
+ res, err := frontend.Client().Get(frontend.URL)
+ if err != nil {
+ t.Fatalf("Get: %v", err)
+ }
+ defer res.Body.Close()
+ body, _ := io.ReadAll(res.Body)
+ if got, want := string(body), content; got != want {
+ t.Errorf("got response %q, want %q", got, want)
+ }
+}
To view, visit change 407214. To unsubscribe, or for help writing mail filters, visit settings.
Reply all
Reply to author
Forward
0 new messages