internal/frontend: handle redirect in main serving path

Handle the logic of the redirect banner in the ordinary serving path,
instead of using a middleware layer.

To make this work, we have to bypass the cache when the redirect
cookie is set, so we don't cache the page with the banner.

For golang/go#44210

Change-Id: I145f058373980ff9e5d3617529e849b25161b390
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/292489
Trust: Jonathan Amsterdam <jba@google.com>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
Reviewed-by: Julie Qiu <julie@golang.org>

cmd/frontend/main.go

@@ -178,7 +178,6 @@ func main() {
 		middleware.AcceptRequests(http.MethodGet, http.MethodPost, http.MethodHead), // accept only GETs, POSTs and HEADs
 		middleware.BetaPkgGoDevRedirect(),
 		middleware.Quota(cfg.Quota, cacheClient),
-		middleware.RedirectedFrom(),
 		middleware.SecureHeaders(!*disableCSP), // must come before any caching for nonces to work
 		middleware.Experiment(experimenter),
 		middleware.LatestVersions(server.GetLatestInfo), // must come before caching for version badge to work

cmd/pkgsite/main.go

@@ -72,7 +72,6 @@ func main() {
 	server.Install(router.Handle, nil, nil)
 
 	mw := middleware.Chain(
-		middleware.RedirectedFrom(),
 		middleware.LatestVersions(server.GetLatestInfo), // must come before caching for version badge to work
 		middleware.Timeout(54*time.Second),
 	)

content/static/css/unit_header.css

@@ -121,7 +121,6 @@
  * middleware/latestversion.go after unit page is launched.
  */
 .UnitHeader-majorVersionBanner--latest,
-.UnitHeader-redirectedFromBanner--none,
 .DetailsHeader-banner--latest {
   display: none;
 }

content/static/html/helpers/_unit_header.tmpl

@@ -4,6 +4,8 @@
   license that can be found in the LICENSE file.
 -->
 
+{{/* . = internal/frontend.UnitPage */}}
+
 {{define "unit_header"}}
   <header class="UnitHeader" role="complementary"
       aria-label="{{if eq .PageType "std"}}module{{else}}{{.PageType}}{{end}} {{.Title}} information">
@@ -36,12 +38,14 @@
           <span class="UnitHeader-badge">{{.}}</span>
         {{end}}
       </div>
-      <div class="UnitHeader-redirectedFromBanner $$GODISCOVERY_REDIRECTEDFROMCLASS$$">
-        <img height="19px" width="16px" class="UnitHeader-detailIcon" src="/static/img/pkg-icon-info_19x16.svg" alt="">
-        <span>
-          Redirected from <span>$$GODISCOVERY_REDIRECTEDFROMPATH$$</span>.
-        </span>
-      </div>
+      {{with .RedirectedFromPath}}
+        <div class="UnitHeader-redirectedFromBanner">
+          <img height="19px" width="16px" class="UnitHeader-detailIcon" src="/static/img/pkg-icon-info_19x16.svg" alt="">
+          <span>
+          Redirected from <span>{{.}}</span>.
+          </span>
+        </div>
+      {{end}}
       <div class="UnitHeader-majorVersionBanner $$GODISCOVERY_LATESTMAJORCLASS$$" data-test-id="UnitHeader-majorVersionBanner">
         <img height="19px" width="16px" class="UnitHeader-detailIcon" src="/static/img/pkg-icon-info_19x16.svg" alt="">
         <span>

internal/frontend/server_test.go

@@ -6,7 +6,6 @@ package frontend
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"io"
 	"net/http"
@@ -1333,19 +1332,20 @@ func TestServer404Redirect(t *testing.T) {
 				r.AddCookie(c)
 				w = httptest.NewRecorder()
 				handler.ServeHTTP(w, r)
-				if err := checkBanner(w.Result().Body, val); err != nil {
-					t.Fatalf("banner: %v", err)
+				err = checkBody(w.Result().Body, in(".UnitHeader-redirectedFromBanner", hasText(val)))
+				if err != nil {
+					t.Fatal(err)
 				}
 				// Visiting the same page again without the cookie should not
 				// display the banner.
 				r = httptest.NewRequest("GET", loc, nil)
 				w = httptest.NewRecorder()
 				handler.ServeHTTP(w, r)
-				if err := checkBanner(w.Result().Body, val); err != errNoBanner {
-					t.Fatalf("banner #2: got %v, want %v", err, errNoBanner)
+				err = checkBody(w.Result().Body, notIn(".UnitHeader-redirectedFromBanner"))
+				if err != nil {
+					t.Fatal(err)
 				}
 			}
-
 		})
 	}
 }
@@ -1359,22 +1359,13 @@ func findCookie(name string, cookies []*http.Cookie) *http.Cookie {
 	return nil
 }
 
-var errNoBanner = errors.New("no redirect banner")
-
-func checkBanner(body io.ReadCloser, path string) error {
+func checkBody(body io.ReadCloser, c htmlcheck.Checker) error {
 	doc, err := html.Parse(body)
 	if err != nil {
 		return err
 	}
 	_ = body.Close()
-
-	if in(".UnitHeader-redirectedFromBanner--none")(doc) == nil {
-		return errNoBanner
-	}
-	if err := in(".UnitHeader-redirectedFromBanner", hasText(path))(doc); err != nil {
-		return err
-	}
-	return nil
+	return c(doc)
 }
 
 func mustRequest(urlPath string, t *testing.T) *http.Request {
@@ -1472,7 +1463,6 @@ func newTestServer(t *testing.T, proxyModules []*proxy.Module, redisClient *redi
 		t.Fatal(err)
 	}
 	mw := middleware.Chain(
-		middleware.RedirectedFrom(),
 		middleware.Experiment(exp),
 		middleware.LatestVersions(s.GetLatestInfo))
 	return s, mw(mux), func() {

internal/frontend/unit.go

@@ -16,6 +16,7 @@ import (
 	"github.com/google/safehtml"
 	"github.com/google/safehtml/uncheckedconversions"
 	"golang.org/x/pkgsite/internal"
+	"golang.org/x/pkgsite/internal/cookie"
 	"golang.org/x/pkgsite/internal/derrors"
 	"golang.org/x/pkgsite/internal/experiment"
 	"golang.org/x/pkgsite/internal/log"
@@ -73,6 +74,11 @@ type UnitPage struct {
 	// Settings contains settings for the selected tab.
 	SelectedTab TabSettings
 
+	// RedirectedFromPath is the path that redirected to the current page.
+	// If non-empty, a "redirected from" banner will be displayed
+	// (see content/static/html/helpers/_unit_header.tmpl).
+	RedirectedFromPath string
+
 	// Details contains data specific to the type of page being rendered.
 	Details interface{}
 }
@@ -129,25 +135,32 @@ func (s *Server) serveUnitPage(ctx context.Context, w http.ResponseWriter, r *ht
 	}
 
 	latestInfo := s.GetLatestInfo(r.Context(), um.Path, um.ModulePath)
+	var redirectPath string
+	redirectPath, err = cookie.Extract(w, r, cookie.AlternativeModuleFlash)
+	if err != nil {
+		// Don't fail, but don't display a banner either.
+		log.Errorf(ctx, "extracting AlternativeModuleFlash cookie: %v", err)
+	}
 	tabSettings := unitTabLookup[tab]
 	title := pageTitle(um)
 	basePage := s.newBasePage(r, title)
 	basePage.AllowWideContent = true
 	lv := linkVersion(um.Version, um.ModulePath)
 	page := UnitPage{
-		basePage:         basePage,
-		Unit:             um,
-		Breadcrumb:       displayBreadcrumb(um, info.requestedVersion),
-		Title:            title,
-		SelectedTab:      tabSettings,
-		URLPath:          constructUnitURL(um.Path, um.ModulePath, info.requestedVersion),
-		CanonicalURLPath: canonicalURLPath(um),
-		DisplayVersion:   displayVersion(um.Version, um.ModulePath),
-		LinkVersion:      lv,
-		LatestURL:        constructUnitURL(um.Path, um.ModulePath, internal.LatestVersion),
-		LatestMinorClass: latestMinorClass(r.Context(), lv, latestInfo),
-		PageLabels:       pageLabels(um),
-		PageType:         pageType(um),
+		basePage:           basePage,
+		Unit:               um,
+		Breadcrumb:         displayBreadcrumb(um, info.requestedVersion),
+		Title:              title,
+		SelectedTab:        tabSettings,
+		URLPath:            constructUnitURL(um.Path, um.ModulePath, info.requestedVersion),
+		CanonicalURLPath:   canonicalURLPath(um),
+		DisplayVersion:     displayVersion(um.Version, um.ModulePath),
+		LinkVersion:        lv,
+		LatestURL:          constructUnitURL(um.Path, um.ModulePath, internal.LatestVersion),
+		LatestMinorClass:   latestMinorClass(r.Context(), lv, latestInfo),
+		PageLabels:         pageLabels(um),
+		PageType:           pageType(um),
+		RedirectedFromPath: redirectPath,
 	}
 
 	// Use GOOS and GOARCH query parameters to create a build context, which

internal/middleware/caching.go

@@ -21,6 +21,7 @@ import (
 	"go.opencensus.io/tag"
 	icache "golang.org/x/pkgsite/internal/cache"
 	"golang.org/x/pkgsite/internal/config"
+	"golang.org/x/pkgsite/internal/cookie"
 	"golang.org/x/pkgsite/internal/log"
 )
 
@@ -132,6 +133,11 @@ func (c *cache) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 			return
 		}
 	}
+	// If the flash cookie is set, bypass the cache.
+	if _, err := r.Cookie(cookie.AlternativeModuleFlash); err == nil {
+		c.delegate.ServeHTTP(w, r)
+		return
+	}
 	ctx := r.Context()
 	key := r.URL.String()
 	start := time.Now()