internal/frontend: do not redirect on unseen paths
When a request is made to a path that we haven't seen before, we should
always return a 404 with the option to fetch that path. A bug is fixed
which previously redirected unknown versions of a path to the most
recent version.
Fixes golang/go#45107
Change-Id: Ic5180d2e40790eadbc07434cc0ffe25179b35194
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/303771
Trust: Julie Qiu <julie@golang.org>
Run-TryBot: Julie Qiu <julie@golang.org>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Jamal Carvalho <jamal@golang.org>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
diff --git a/internal/frontend/404.go b/internal/frontend/404.go
index 6227084..1ca29a2 100644
--- a/internal/frontend/404.go
+++ b/internal/frontend/404.go
@@ -67,18 +67,21 @@
fr, err := previousFetchStatusAndResponse(ctx, db, fullPath, modulePath, requestedVersion)
if err != nil {
+ // If an error occurred, it means that we have never tried to fetch
+ // this path before or an error occurred when we tried to
+ // gather data about this 404.
+ //
+ // If the latter, log the error.
+ // In either case, give the user the option to fetch that path.
if !errors.Is(err, derrors.NotFound) {
log.Error(ctx, err)
}
- // Redirect to the search result page for an empty directory that is above nested modules.
- // For golang/go#43725
- nm, err := ds.GetNestedModules(ctx, fullPath)
- if err == nil && len(nm) > 0 {
- http.Redirect(w, r, "/search?q="+url.QueryEscape(fullPath), http.StatusFound)
- return nil
- }
return pathNotFoundError(fullPath, requestedVersion)
}
+
+ // If we've reached this point, we know that we've seen this path before.
+ // Show a relevant page or redirect the use based on the previous fetch
+ // response.
switch fr.status {
case http.StatusFound, derrors.ToStatus(derrors.AlternativeModule):
if fr.goModPath == fullPath {
@@ -105,7 +108,7 @@
return
}
// Redirect to the search result page for an empty directory that is above nested modules.
- // For golang/go#43725
+ // See https://golang.org/issue/43725 for context.
nm, err := ds.GetNestedModules(ctx, fullPath)
if err == nil && len(nm) > 0 {
http.Redirect(w, r, "/search?q="+url.QueryEscape(fullPath), http.StatusFound)
diff --git a/internal/frontend/server_test.go b/internal/frontend/server_test.go
index b994ed1..3231e99 100644
--- a/internal/frontend/server_test.go
+++ b/internal/frontend/server_test.go
@@ -1592,18 +1592,37 @@
defer postgres.ResetTestDB(testDB, t)
postgres.MustInsertModule(ctx, t, testDB, sample.Module(sample.ModulePath, sample.VersionString, ""))
- postgres.MustInsertModule(ctx, t, testDB, sample.Module(sample.ModulePath+"/missing/c", sample.VersionString, ""))
+ postgres.MustInsertModule(ctx, t, testDB, sample.Module(sample.ModulePath+"/missing/dir/c", sample.VersionString, ""))
+
+ missingPath := sample.ModulePath + "/missing"
+ notInsertedPath := sample.ModulePath + "/missing/dir"
+ if err := testDB.UpsertVersionMap(ctx, &internal.VersionMap{
+ ModulePath: missingPath,
+ RequestedVersion: internal.LatestVersion,
+ ResolvedVersion: sample.VersionString,
+ }); err != nil {
+ t.Fatal(err)
+ }
_, handler, _ := newTestServer(t, nil, nil)
- w := httptest.NewRecorder()
- dirPath := sample.ModulePath + "/missing"
- handler.ServeHTTP(w, httptest.NewRequest("GET", "/"+dirPath, nil))
- if w.Code != http.StatusFound {
- t.Errorf("%q: got status code = %d, want %d", "/"+dirPath, w.Code, http.StatusFound)
- }
- got := w.Header().Get("Location")
- wantURL := "/search?q=" + url.PathEscape(dirPath)
- if got != wantURL {
- t.Errorf("got location = %q, want %q", got, wantURL)
+ for _, test := range []struct {
+ name, path string
+ wantStatus int
+ wantLocation string
+ }{
+ {"want 404 for unknown version of module", sample.ModulePath + "@v0.5.0", http.StatusNotFound, ""},
+ {"want 404 for never fetched directory", notInsertedPath, http.StatusNotFound, ""},
+ {"want 302 for previously fetched directory", missingPath, http.StatusFound, "/search?q=" + url.PathEscape(missingPath)},
+ } {
+ t.Run(test.name, func(t *testing.T) {
+ w := httptest.NewRecorder()
+ handler.ServeHTTP(w, httptest.NewRequest("GET", "/"+test.path, nil))
+ if w.Code != test.wantStatus {
+ t.Errorf("%q: got status code = %d, want %d", "/"+test.path, w.Code, test.wantStatus)
+ }
+ if got := w.Header().Get("Location"); got != test.wantLocation {
+ t.Errorf("got location = %q, want %q", got, test.wantLocation)
+ }
+ })
}
}
