Merge pull request #1158 from tw4452852/1154

proxy: handle 'without' option in encoded form of URL path

caddyhttp/proxy/proxy_test.go

@@ -592,7 +592,7 @@ func TestMultiReverseProxyFromClient(t *testing.T) {
 
 	for _, tt := range multiProxy {
 		// Create client request
-		reqURL := singleJoiningSlash(proxy.URL, tt.url)
+		reqURL := proxy.URL + tt.url
 		req, err := http.NewRequest("GET", reqURL, nil)
 		if err != nil {
 			t.Fatalf("Failed to create request: %v", err)
@@ -747,6 +747,70 @@ func basicAuthTestcase(t *testing.T, upstreamUser, clientUser *url.Userinfo) {
 	}
 }
 
+func TestProxyDirectorURL(t *testing.T) {
+	for i, c := range []struct {
+		requestURL string
+		targetURL  string
+		without    string
+		expectURL  string
+	}{
+		{
+			requestURL: `http://localhost:2020/test`,
+			targetURL:  `https://localhost:2021`,
+			expectURL:  `https://localhost:2021/test`,
+		},
+		{
+			requestURL: `http://localhost:2020/test`,
+			targetURL:  `https://localhost:2021/t`,
+			expectURL:  `https://localhost:2021/t/test`,
+		},
+		{
+			requestURL: `http://localhost:2020/test?t=w`,
+			targetURL:  `https://localhost:2021/t`,
+			expectURL:  `https://localhost:2021/t/test?t=w`,
+		},
+		{
+			requestURL: `http://localhost:2020/test`,
+			targetURL:  `https://localhost:2021/t?foo=bar`,
+			expectURL:  `https://localhost:2021/t/test?foo=bar`,
+		},
+		{
+			requestURL: `http://localhost:2020/test?t=w`,
+			targetURL:  `https://localhost:2021/t?foo=bar`,
+			expectURL:  `https://localhost:2021/t/test?foo=bar&t=w`,
+		},
+		{
+			requestURL: `http://localhost:2020/test?t=w`,
+			targetURL:  `https://localhost:2021/t?foo=bar`,
+			expectURL:  `https://localhost:2021/t?foo=bar&t=w`,
+			without:    "/test",
+		},
+		{
+			requestURL: `http://localhost:2020/test?t%3dw`,
+			targetURL:  `https://localhost:2021/t?foo%3dbar`,
+			expectURL:  `https://localhost:2021/t?foo%3dbar&t%3dw`,
+			without:    "/test",
+		},
+	} {
+		targetURL, err := url.Parse(c.targetURL)
+		if err != nil {
+			t.Errorf("case %d failed to parse target URL: %s", i, err)
+			continue
+		}
+		req, err := http.NewRequest("GET", c.requestURL, nil)
+		if err != nil {
+			t.Errorf("case %d failed to create request: %s", i, err)
+			continue
+		}
+
+		NewSingleHostReverseProxy(targetURL, c.without, 0).Director(req)
+		if expect, got := c.expectURL, req.URL.String(); expect != got {
+			t.Errorf("case %d url not equal: expect %q, but got %q",
+				i, expect, got)
+		}
+	}
+}
+
 func newFakeUpstream(name string, insecure bool) *fakeUpstream {
 	uri, _ := url.Parse(name)
 	u := &fakeUpstream{

caddyhttp/proxy/reverseproxy.go

@@ -17,6 +17,7 @@ import (
 	"net"
 	"net/http"
 	"net/url"
+	"path"
 	"strings"
 	"sync"
 	"time"
@@ -53,18 +54,6 @@ type ReverseProxy struct {
 	FlushInterval time.Duration
 }
 
-func singleJoiningSlash(a, b string) string {
-	aslash := strings.HasSuffix(a, "/")
-	bslash := strings.HasPrefix(b, "/")
-	switch {
-	case aslash && bslash:
-		return a + b[1:]
-	case !aslash && !bslash:
-		return a + "/" + b
-	}
-	return a + b
-}
-
 // Though the relevant directive prefix is just "unix:", url.Parse
 // will - assuming the regular URL scheme - add additional slashes
 // as if "unix" was a request protocol.
@@ -95,12 +84,19 @@ func NewSingleHostReverseProxy(target *url.URL, without string, keepalive int) *
 			req.URL.Scheme = target.Scheme
 			req.URL.Host = target.Host
 		}
-		req.URL.Path = singleJoiningSlash(target.Path, req.URL.Path)
+
-		if targetQuery == "" || req.URL.RawQuery == "" {
+		// We should remove the `without` prefix at first.
-			req.URL.RawQuery = targetQuery + req.URL.RawQuery
+		if without != "" {
-		} else {
+			req.URL.Path = strings.TrimPrefix(req.URL.Path, without)
-			req.URL.RawQuery = targetQuery + "&" + req.URL.RawQuery
+			if req.URL.Opaque != "" {
+				req.URL.Opaque = strings.TrimPrefix(req.URL.Opaque, without)
 			}
+			if req.URL.RawPath != "" {
+				req.URL.RawPath = strings.TrimPrefix(req.URL.RawPath, without)
+			}
+		}
+
+		req.URL.Path = path.Join(target.Path, req.URL.Path)
 		// Trims the path of the socket from the URL path.
 		// This is done because req.URL passed to your proxied service
 		// will have the full path of the socket file prefixed to it.
@@ -112,9 +108,11 @@ func NewSingleHostReverseProxy(target *url.URL, without string, keepalive int) *
 			socketPrefix := target.String()[len("unix://"):]
 			req.URL.Path = strings.TrimPrefix(req.URL.Path, socketPrefix)
 		}
-		// We are then safe to remove the `without` prefix.
+
-		if without != "" {
+		if targetQuery == "" || req.URL.RawQuery == "" {
-			req.URL.Path = strings.TrimPrefix(req.URL.Path, without)
+			req.URL.RawQuery = targetQuery + req.URL.RawQuery
+		} else {
+			req.URL.RawQuery = targetQuery + "&" + req.URL.RawQuery
 		}
 	}
 	rp := &ReverseProxy{Director: director, FlushInterval: 250 * time.Millisecond} // flushing good for streaming & server-sent events