internal/frontend: fix redirect loops
Fixes golang/go#44534
Change-Id: I5ce5977d65941739174845969fc9215a11ee7218
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/295549
Trust: Julie Qiu <julie@golang.org>
Run-TryBot: Julie Qiu <julie@golang.org>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
diff --git a/internal/frontend/404.go b/internal/frontend/404.go
index 76a34b4..6227084 100644
--- a/internal/frontend/404.go
+++ b/internal/frontend/404.go
@@ -81,10 +81,22 @@
}
switch fr.status {
case http.StatusFound, derrors.ToStatus(derrors.AlternativeModule):
+ if fr.goModPath == fullPath {
+ // The redirectPath and the fullpath are the same. Do not redirect
+ // to avoid ending up in a loop.
+ return errUnitNotFoundWithoutFetch
+ }
+ vm, err := db.GetVersionMap(ctx, fr.goModPath, internal.LatestVersion)
+ if (err != nil && !errors.Is(err, derrors.NotFound)) ||
+ (vm != nil && vm.Status != http.StatusOK) {
+ // We attempted to fetch the canonical module path before and were
+ // not successful. Do not redirect this request.
+ return errUnitNotFoundWithoutFetch
+ }
u := constructUnitURL(fr.goModPath, fr.goModPath, internal.LatestVersion)
cookie.Set(w, cookie.AlternativeModuleFlash, fullPath, u)
http.Redirect(w, r, u, http.StatusFound)
- return
+ return nil
case http.StatusInternalServerError:
return pathNotFoundError(fullPath, requestedVersion)
default:
diff --git a/internal/frontend/server_test.go b/internal/frontend/server_test.go
index ff84e5b..277830e 100644
--- a/internal/frontend/server_test.go
+++ b/internal/frontend/server_test.go
@@ -1239,6 +1239,62 @@
}
}
+func TestServer404Redirect_NoLoop(t *testing.T) {
+ ctx, cancel := context.WithTimeout(context.Background(), testTimeout)
+ defer cancel()
+
+ altPath := "module.path/alternative"
+ goModPath := "module.path/alternative/pkg"
+ defer postgres.ResetTestDB(testDB, t)
+ sampleModule := sample.DefaultModule()
+ postgres.MustInsertModule(ctx, t, testDB, sampleModule)
+ alternativeModule := &internal.VersionMap{
+ ModulePath: altPath,
+ GoModPath: goModPath,
+ RequestedVersion: internal.LatestVersion,
+ ResolvedVersion: sample.VersionString,
+ Status: derrors.ToStatus(derrors.AlternativeModule),
+ }
+ alternativeModulePkg := &internal.VersionMap{
+ ModulePath: goModPath,
+ GoModPath: goModPath,
+ RequestedVersion: internal.LatestVersion,
+ ResolvedVersion: sample.VersionString,
+ Status: http.StatusNotFound,
+ }
+ if err := testDB.UpsertVersionMap(ctx, alternativeModule); err != nil {
+ t.Fatal(err)
+ }
+ if err := testDB.UpsertVersionMap(ctx, alternativeModulePkg); err != nil {
+ t.Fatal(err)
+ }
+
+ rs, err := miniredis.Run()
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer rs.Close()
+
+ _, handler, _ := newTestServer(t, nil, redis.NewClient(&redis.Options{Addr: rs.Addr()}))
+
+ for _, test := range []struct {
+ name, path string
+ status int
+ }{
+ {"do not redirect if alternative module does not successfully return", "/" + altPath, http.StatusNotFound},
+ {"do not redirect go mod path endlessly", "/" + goModPath, http.StatusNotFound},
+ } {
+ t.Run(test.name, func(t *testing.T) {
+ w := httptest.NewRecorder()
+ handler.ServeHTTP(w, httptest.NewRequest("GET", test.path, nil))
+ // Check for http.StatusFound, which indicates a redirect.
+ if w.Code != test.status {
+ t.Errorf("%q: got status code = %d, want %d", test.path, w.Code, test.status)
+ }
+ })
+ }
+}
+
// Verify that some paths that aren't found will redirect to valid pages.
// Sometimes redirection sets the AlternativeModuleFlash cookie and puts
// up a banner.
