From faa5248d1f1c73a1b5ed61a5a321319adae04f5f Mon Sep 17 00:00:00 2001
From: Toby Allen <tobyallen@toflidium.com>
Date: Fri, 16 Feb 2018 21:18:02 +0000
Subject: [PATCH] httpserver: Leave %2f encoded when trimming path in site
 address Fix #1927 (#2014)

* Trim path prefix using EscapedPath()

* clarify comments

* Added Tests for trimPathPrefix

* Ensure path with trailing slash is properly trimmed

* Updated tests to match prepatch behaviour

* Updated tests to match prepatch behaviour

* call parse on url rather than instance

* add additional tests

* return unmodified url if error.  Additional tests
---
 caddyhttp/httpserver/server.go      | 20 +++++--
 caddyhttp/httpserver/server_test.go | 89 +++++++++++++++++++++++++++++
 2 files changed, 105 insertions(+), 4 deletions(-)

diff --git a/caddyhttp/httpserver/server.go b/caddyhttp/httpserver/server.go
index 9d3ae038..ab65d955 100644
--- a/caddyhttp/httpserver/server.go
+++ b/caddyhttp/httpserver/server.go
@@ -413,15 +413,27 @@ func (s *Server) serveHTTP(w http.ResponseWriter, r *http.Request) (int, error)
 	// the URL path, so a request to example.com/foo/blog on the site
 	// defined as example.com/foo appears as /blog instead of /foo/blog.
 	if pathPrefix != "/" {
-		r.URL.Path = strings.TrimPrefix(r.URL.Path, pathPrefix)
-		if !strings.HasPrefix(r.URL.Path, "/") {
-			r.URL.Path = "/" + r.URL.Path
-		}
+		r.URL = trimPathPrefix(r.URL, pathPrefix)
 	}
 
 	return vhost.middlewareChain.ServeHTTP(w, r)
 }
 
+func trimPathPrefix(u *url.URL, prefix string) *url.URL {
+	// We need to use URL.EscapedPath() when trimming the pathPrefix as
+	// URL.Path is ambiguous about / or %2f - see docs. See #1927
+	trimmed := strings.TrimPrefix(u.EscapedPath(), prefix)
+	if !strings.HasPrefix(trimmed, "/") {
+		trimmed = "/" + trimmed
+	}
+	trimmedURL, err := url.Parse(trimmed)
+	if err != nil {
+		log.Printf("[ERROR] Unable to parse trimmed URL %s: %v", trimmed, err)
+		return u
+	}
+	return trimmedURL
+}
+
 // Address returns the address s was assigned to listen on.
 func (s *Server) Address() string {
 	return s.Server.Addr
diff --git a/caddyhttp/httpserver/server_test.go b/caddyhttp/httpserver/server_test.go
index a781a80a..82926851 100644
--- a/caddyhttp/httpserver/server_test.go
+++ b/caddyhttp/httpserver/server_test.go
@@ -16,6 +16,7 @@ package httpserver
 
 import (
 	"net/http"
+	"net/url"
 	"testing"
 	"time"
 )
@@ -126,6 +127,94 @@ func TestMakeHTTPServerWithTimeouts(t *testing.T) {
 	}
 }
 
+func TestTrimPathPrefix(t *testing.T) {
+	for i, pt := range []struct {
+		path       string
+		prefix     string
+		expected   string
+		shouldFail bool
+	}{
+		{
+			path:       "/my/path",
+			prefix:     "/my",
+			expected:   "/path",
+			shouldFail: false,
+		},
+		{
+			path:       "/my/%2f/path",
+			prefix:     "/my",
+			expected:   "/%2f/path",
+			shouldFail: false,
+		},
+		{
+			path:       "/my/path",
+			prefix:     "/my/",
+			expected:   "/path",
+			shouldFail: false,
+		},
+		{
+			path:       "/my///path",
+			prefix:     "/my",
+			expected:   "/path",
+			shouldFail: true,
+		},
+		{
+			path:       "/my///path",
+			prefix:     "/my",
+			expected:   "///path",
+			shouldFail: false,
+		},
+		{
+			path:       "/my/path///slash",
+			prefix:     "/my",
+			expected:   "/path///slash",
+			shouldFail: false,
+		},
+		{
+			path:       "/my/%2f/path/%2f",
+			prefix:     "/my",
+			expected:   "/%2f/path/%2f",
+			shouldFail: false,
+		}, {
+			path:       "/my/%20/path",
+			prefix:     "/my",
+			expected:   "/%20/path",
+			shouldFail: false,
+		}, {
+			path:       "/path",
+			prefix:     "",
+			expected:   "/path",
+			shouldFail: false,
+		}, {
+			path:       "/path/my/",
+			prefix:     "/my",
+			expected:   "/path/my/",
+			shouldFail: false,
+		}, {
+			path:       "",
+			prefix:     "/my",
+			expected:   "/",
+			shouldFail: false,
+		}, {
+			path:       "/apath",
+			prefix:     "",
+			expected:   "/apath",
+			shouldFail: false,
+		},
+	} {
+
+		u, _ := url.Parse(pt.path)
+		if got, want := trimPathPrefix(u, pt.prefix), pt.expected; got.EscapedPath() != want {
+			if !pt.shouldFail {
+
+				t.Errorf("Test %d: Expected='%s', but was '%s' ", i, want, got.EscapedPath())
+			}
+		} else if pt.shouldFail {
+			t.Errorf("SHOULDFAIL Test %d: Expected='%s', and was '%s' but should fail", i, want, got.EscapedPath())
+		}
+	}
+}
+
 func TestMakeHTTPServerWithHeaderLimit(t *testing.T) {
 	for name, c := range map[string]struct {
 		group  []*SiteConfig