From 9d0a61e25e0501f9825dd32cdf841b6b9a5789d7 Mon Sep 17 00:00:00 2001 From: Marcel Ludwig Date: Thu, 8 Oct 2020 15:55:21 +0200 Subject: [PATCH 1/2] Fix wildcard path join with trailing slash and respect req path --- handler/proxy.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/handler/proxy.go b/handler/proxy.go index bba6f458e..1b6bbf782 100644 --- a/handler/proxy.go +++ b/handler/proxy.go @@ -284,6 +284,11 @@ func (p *Proxy) director(req *http.Request) { if pathMatch, ok := req.Context(). Value(request.Wildcard).(string); ok && strings.HasSuffix(p.options.Path, "/**") { + if pathMatch == "" { // wildcard "root" hit, take a look if the req has a trailing slash and apply + if req.URL.Path[len(req.URL.Path)-1] == '/' { + pathMatch = "/" + } + } req.URL.Path = utils.JoinPath(strings.ReplaceAll(p.options.Path, "/**", "/"), pathMatch) } else if p.options.Path != "" { req.URL.Path = p.options.Path From 8f8d62dc0c7297bb26fae14ecf96092139efb995 Mon Sep 17 00:00:00 2001 From: Marcel Ludwig Date: Thu, 8 Oct 2020 16:26:31 +0200 Subject: [PATCH 2/2] Refine related tests --- handler/proxy.go | 2 +- handler/proxy_test.go | 35 ++++++++++++++++++++--------------- 2 files changed, 21 insertions(+), 16 deletions(-) diff --git a/handler/proxy.go b/handler/proxy.go index 1b6bbf782..303d6258f 100644 --- a/handler/proxy.go +++ b/handler/proxy.go @@ -284,7 +284,7 @@ func (p *Proxy) director(req *http.Request) { if pathMatch, ok := req.Context(). Value(request.Wildcard).(string); ok && strings.HasSuffix(p.options.Path, "/**") { - if pathMatch == "" { // wildcard "root" hit, take a look if the req has a trailing slash and apply + if pathMatch == "" && req.URL.Path != "" { // wildcard "root" hit, take a look if the req has a trailing slash and apply if req.URL.Path[len(req.URL.Path)-1] == '/' { pathMatch = "/" } diff --git a/handler/proxy_test.go b/handler/proxy_test.go index 30e81a36c..a295ba4e9 100644 --- a/handler/proxy_test.go +++ b/handler/proxy_test.go @@ -382,47 +382,52 @@ func TestProxy_ServeHTTP_CORS(t *testing.T) { } func TestProxy_director(t *testing.T) { - defaultReq := httptest.NewRequest("GET", "http://example.com", nil) - log, _ := logrustest.NewNullLogger() + nullLog := log.WithContext(nil) type fields struct { - evalContext *hcl.EvalContext - log *logrus.Entry - options *ProxyOptions + log *logrus.Entry + options *ProxyOptions } emptyOptions := []hcl.Body{hcl.EmptyBody()} + bgCtx := context.Background() tests := []struct { name string fields fields - req *http.Request + path string + ctx context.Context expReq *http.Request }{ - {"proxy url settings", fields{eval.NewENVContext(nil), log.WithContext(nil), &ProxyOptions{Origin: "http://1.2.3.4", Context: emptyOptions}}, defaultReq, httptest.NewRequest("GET", "http://1.2.3.4", nil)}, - {"proxy url settings w/hostname", fields{eval.NewENVContext(nil), log.WithContext(nil), &ProxyOptions{Origin: "http://1.2.3.4", Hostname: "couper.io", Context: emptyOptions}}, defaultReq, httptest.NewRequest("GET", "http://couper.io", nil)}, - {"proxy url settings w/wildcard ctx", fields{eval.NewENVContext(nil), log.WithContext(nil), &ProxyOptions{Origin: "http://1.2.3.4", Hostname: "couper.io", Path: "/**", Context: emptyOptions}}, defaultReq.WithContext(context.WithValue(defaultReq.Context(), request.Wildcard, "/hans")), httptest.NewRequest("GET", "http://couper.io/hans", nil)}, - {"proxy url settings w/wildcard ctx trailing slash", fields{eval.NewENVContext(nil), log.WithContext(nil), &ProxyOptions{Origin: "http://1.2.3.4", Hostname: "couper.io", Path: "/**", Context: emptyOptions}}, defaultReq.WithContext(context.WithValue(defaultReq.Context(), request.Wildcard, "/docs/")), httptest.NewRequest("GET", "http://couper.io/docs/", nil)}, + {"proxy url settings", fields{nullLog, &ProxyOptions{Origin: "http://1.2.3.4", Context: emptyOptions}}, "", bgCtx, httptest.NewRequest("GET", "http://1.2.3.4", nil)}, + {"proxy url settings w/hostname", fields{nullLog, &ProxyOptions{Origin: "http://1.2.3.4", Hostname: "couper.io", Context: emptyOptions}}, "", bgCtx, httptest.NewRequest("GET", "http://couper.io", nil)}, + {"proxy url settings w/wildcard ctx", fields{nullLog, &ProxyOptions{Origin: "http://1.2.3.4", Hostname: "couper.io", Path: "/**", Context: emptyOptions}}, "/peter", context.WithValue(bgCtx, request.Wildcard, "/hans"), httptest.NewRequest("GET", "http://couper.io/hans", nil)}, + {"proxy url settings w/wildcard ctx empty", fields{nullLog, &ProxyOptions{Origin: "http://1.2.3.4", Hostname: "couper.io", Path: "/docs/**", Context: emptyOptions}}, "", context.WithValue(bgCtx, request.Wildcard, ""), httptest.NewRequest("GET", "http://couper.io/docs", nil)}, + {"proxy url settings w/wildcard ctx empty /w trailing path slash", fields{nullLog, &ProxyOptions{Origin: "http://1.2.3.4", Hostname: "couper.io", Path: "/docs/**", Context: emptyOptions}}, "/", context.WithValue(bgCtx, request.Wildcard, ""), httptest.NewRequest("GET", "http://couper.io/docs/", nil)}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - p, err := NewProxy(tt.fields.options, tt.fields.log, tt.fields.evalContext) + p, err := NewProxy(tt.fields.options, tt.fields.log, eval.NewENVContext(nil)) if err != nil { t.Fatal(err) } + + req := httptest.NewRequest(http.MethodGet, "https://example.com"+tt.path, nil) + *req = *req.Clone(tt.ctx) + proxy := p.(*Proxy) - proxy.director(tt.req) + proxy.director(req) if tt.fields.options.Hostname != "" && tt.fields.options.Hostname != tt.expReq.Host { t.Errorf("expected same host value, want: %q, got: %q", tt.fields.options.Hostname, tt.expReq.Host) - } else if tt.fields.options.Hostname == "" && tt.req.Host != tt.expReq.Host { + } else if tt.fields.options.Hostname == "" && req.Host != tt.expReq.Host { t.Error("expected a configured request host") } - if tt.req.URL.Path != tt.expReq.URL.Path { - t.Errorf("expected path: %q, got: %q", tt.expReq.URL.Path, tt.req.URL.Path) + if req.URL.Path != tt.expReq.URL.Path { + t.Errorf("expected path: %q, got: %q", tt.expReq.URL.Path, req.URL.Path) } }) }