Merge pull request #1378 from tw4452852/1362

proxy: handle encoded path in URL

caddyhttp/proxy/proxy.go

@@ -273,11 +273,6 @@ func createUpstreamRequest(r *http.Request) *http.Request {
 		outreq.Body = nil
 	}
 
-	// Restore URL Path if it has been modified
-	if outreq.URL.RawPath != "" {
-		outreq.URL.Opaque = outreq.URL.RawPath
-	}
-
 	// We are modifying the same underlying map from req (shallow
 	// copied above) so we only copy it if necessary.
 	copiedHeaders := false

caddyhttp/proxy/proxy_test.go

@@ -949,6 +949,26 @@ func TestProxyDirectorURL(t *testing.T) {
 			expectURL:  `https://localhost:2021/t/mypath`,
 			without:    "/test",
 		},
+		{
+			requestURL: `http://localhost:2020/%2C`,
+			targetURL:  `https://localhost:2021/t/`,
+			expectURL:  `https://localhost:2021/t/%2C`,
+		},
+		{
+			requestURL: `http://localhost:2020/%2C/`,
+			targetURL:  `https://localhost:2021/t/`,
+			expectURL:  `https://localhost:2021/t/%2C/`,
+		},
+		{
+			requestURL: `http://localhost:2020/test`,
+			targetURL:  `https://localhost:2021/%2C`,
+			expectURL:  `https://localhost:2021/%2C/test`,
+		},
+		{
+			requestURL: `http://localhost:2020/%2C`,
+			targetURL:  `https://localhost:2021/%2C`,
+			expectURL:  `https://localhost:2021/%2C/%2C`,
+		},
 	} {
 		targetURL, err := url.Parse(c.targetURL)
 		if err != nil {

caddyhttp/proxy/reverseproxy.go

@@ -18,7 +18,6 @@ import (
 	"net"
 	"net/http"
 	"net/url"
-	"path"
 	"strings"
 	"sync"
 	"time"
@@ -89,6 +88,18 @@ func socketDial(hostName string) func(network, addr string) (conn net.Conn, err
 	}
 }
 
+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 && b != "":
+		return a + "/" + b
+	}
+	return a + b
+}
+
 // NewSingleHostReverseProxy returns a new ReverseProxy that rewrites
 // 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",
@@ -119,12 +130,31 @@ func NewSingleHostReverseProxy(target *url.URL, without string, keepalive int) *
 			}
 		}
 
-		hadTrailingSlash := strings.HasSuffix(req.URL.Path, "/")
-		req.URL.Path = path.Join(target.Path, req.URL.Path)
-		// path.Join will strip off the last /, so put it back if it was there.
-		if hadTrailingSlash && !strings.HasSuffix(req.URL.Path, "/") {
-			req.URL.Path = req.URL.Path + "/"
+		// prefer returns val if it isn't empty, otherwise def
+		prefer := func(val, def string) string {
+			if val != "" {
+				return val
+			}
+			return def
+		}
+		// Make up the final URL by concatenating the request and target URL.
+		//
+		// If there is encoded part in request or target URL,
+		// the final URL should also be in encoded format.
+		// Here, we concatenate their encoded parts which are stored
+		// in URL.Opaque and URL.RawPath, if it is empty use
+		// URL.Path instead.
+		if req.URL.Opaque != "" || target.Opaque != "" {
+			req.URL.Opaque = singleJoiningSlash(
+				prefer(target.Opaque, target.Path),
+				prefer(req.URL.Opaque, req.URL.Path))
 		}
+		if req.URL.RawPath != "" || target.RawPath != "" {
+			req.URL.RawPath = singleJoiningSlash(
+				prefer(target.RawPath, target.Path),
+				prefer(req.URL.RawPath, req.URL.Path))
+		}
+		req.URL.Path = singleJoiningSlash(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
@@ -136,6 +166,12 @@ func NewSingleHostReverseProxy(target *url.URL, without string, keepalive int) *
 			// See comment on socketDial for the trim
 			socketPrefix := target.String()[len("unix://"):]
 			req.URL.Path = strings.TrimPrefix(req.URL.Path, socketPrefix)
+			if req.URL.Opaque != "" {
+				req.URL.Opaque = strings.TrimPrefix(req.URL.Opaque, socketPrefix)
+			}
+			if req.URL.RawPath != "" {
+				req.URL.RawPath = strings.TrimPrefix(req.URL.RawPath, socketPrefix)
+			}
 		}
 
 		if targetQuery == "" || req.URL.RawQuery == "" {