Fix sumdb/* paths when config.PathPrefix is set (#1620)

* Fix sumdb/* paths when config.PathPrefix is set http.StripPrefix will look at the entire request path when called, if we do not include config.PathPrefix then the StripPrefix call will never receive a valid path from the application and the user will always get a 404 error. There were no test where I could easily check this regression so I also added a few endpoint tests, the last test will fail with a 404 instead of 403 if this change in not applied. * Update cmd/proxy/actions/app_proxy.go Co-authored-by: Marwan Sulaiman <marwan-at-work@github.com> * Update cmd/proxy/actions/app_proxy_test.go Co-authored-by: Marwan Sulaiman <marwan-at-work@github.com> * Update cmd/proxy/actions/app_proxy_test.go Co-authored-by: Marwan Sulaiman <marwan-at-work@github.com> * Update cmd/proxy/actions/app_proxy_test.go Co-authored-by: Marwan Sulaiman <marwan-at-work@github.com> * Removed unneeded import of logrus Co-authored-by: Marwan Sulaiman <marwan-at-work@github.com>
2026-02-03 11:00:32 +00:00 · 2020-06-05 12:01:38 -04:00
parent 28d606947a
commit c08aa890cb
2 changed files with 92 additions and 1 deletions
@@ -47,7 +47,7 @@ func addProxyRoutes(
 		sumHandler := sumdbProxy(sumdbURL, c.NoSumPatterns)
 		pathPrefix := "/sumdb/" + sumdbURL.Host
 		r.PathPrefix(pathPrefix + "/").Handler(
-			http.StripPrefix(pathPrefix, sumHandler),
+			http.StripPrefix(strings.TrimSuffix(c.PathPrefix, "/")+pathPrefix, sumHandler),
 		)
 	}

@@ -0,0 +1,91 @@
+package actions
+
+import (
+	"encoding/json"
+	"io/ioutil"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+
+	"github.com/gomods/athens/pkg/build"
+	"github.com/gomods/athens/pkg/config"
+	"github.com/gomods/athens/pkg/log"
+	"github.com/gomods/athens/pkg/storage/mem"
+	"github.com/gorilla/mux"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+type routeTest struct {
+	method string
+	path   string
+	body   string
+	test   func(t *testing.T, resp *http.Response)
+}
+
+func TestProxyRoutes(t *testing.T) {
+	r := mux.NewRouter()
+	s, err := mem.NewStorage()
+	require.NoError(t, err)
+	l := log.NoOpLogger()
+	c, err := config.Load("")
+	require.NoError(t, err)
+	c.NoSumPatterns = []string{"*"} // catch all patterns with noSumWrapper to ensure the sumdb handler doesn't make a real http request to the sumdb server.
+	c.PathPrefix = "/prefix"
+	subRouter := r.PathPrefix(c.PathPrefix).Subrouter()
+	err = addProxyRoutes(subRouter, s, l, c)
+	require.NoError(t, err)
+
+	baseURL := "https://athens.azurefd.net" + c.PathPrefix
+
+	testCases := []routeTest{
+		{"GET", "/", "", func(t *testing.T, resp *http.Response) {
+			assert.Equal(t, http.StatusOK, resp.StatusCode)
+			body, err := ioutil.ReadAll(resp.Body)
+			require.NoError(t, err)
+			assert.Equal(t, `"Welcome to The Athens Proxy"`, string(body))
+		}},
+		{"GET", "/badz", "", func(t *testing.T, resp *http.Response) {
+			assert.Equal(t, http.StatusNotFound, resp.StatusCode)
+		}},
+		{"GET", "/healthz", "", func(t *testing.T, resp *http.Response) {
+			assert.Equal(t, http.StatusOK, resp.StatusCode)
+		}},
+		{"GET", "/readyz", "", func(t *testing.T, resp *http.Response) {
+			assert.Equal(t, http.StatusOK, resp.StatusCode)
+		}},
+		{"GET", "/version", "", func(t *testing.T, resp *http.Response) {
+			assert.Equal(t, http.StatusOK, resp.StatusCode)
+			details := build.Details{}
+			err := json.NewDecoder(resp.Body).Decode(&details)
+			require.NoError(t, err)
+			assert.EqualValues(t, build.Data(), details)
+		}},
+
+		// Default sumdb is sum.golang.org
+		{"GET", "/sumdb/sum.golang.org/supported", "", func(t *testing.T, resp *http.Response) {
+			assert.Equal(t, http.StatusOK, resp.StatusCode)
+		}},
+		{"GET", "/sumdb/sum.rust-lang.org/supported", "", func(t *testing.T, resp *http.Response) {
+			assert.Equal(t, http.StatusNotFound, resp.StatusCode)
+		}},
+		{"GET", "/sumdb/sum.golang.org/lookup/github.com/gomods/athens", "", func(t *testing.T, resp *http.Response) {
+			assert.Equal(t, http.StatusForbidden, resp.StatusCode)
+		}},
+	}
+
+	for _, tc := range testCases {
+		req := httptest.NewRequest(
+			tc.method,
+			baseURL+tc.path,
+			strings.NewReader(tc.body),
+		)
+		t.Run(req.RequestURI, func(t *testing.T) {
+			w := httptest.NewRecorder()
+			r.ServeHTTP(w, req)
+			tc.test(t, w.Result())
+		})
+	}
+
+}