diff --git a/docs/services/caching.md b/docs/services/caching.md index febebbd..295dc39 100644 --- a/docs/services/caching.md +++ b/docs/services/caching.md @@ -126,7 +126,7 @@ The kind names what was saved (a plugin picks it, for example `faq`, `menu`, `me `cooler.PurgeSiteCacheAfterCommit` registers the request on the save's transaction through `lagoon.AfterCommit`. It is sent only after the commit, so a page view between the purge and the commit cannot re-cache the old content, and a save that fails or is rolled back sends nothing. Outside a transaction it is sent at once. -A purge never fails a save. The request is bounded by `cooler.SiteCachePurgeTimeout` (2 s) and detached from the admin request's cancellation; an unreachable site, an error status or a timeout is logged as one `cooler: site cache purge failed` warning and the save answers as usual. +A purge never fails a save. The request is bounded by `cooler.SiteCachePurgeTimeout` (2 s) and detached from the admin request's cancellation; an unreachable site, an error status or a timeout is logged as one `cooler: site cache purge failed` warning and the save answers as usual. Redirects are not followed: a 3xx answer is logged as a failed purge with its status, so the bearer token never goes to another URL and the POST never turns into a GET. Point `site_cache.purge_url` at the final URL. ### Purging from a plugin diff --git a/modules/cooler/README.md b/modules/cooler/README.md index b4f2413..5361a39 100644 --- a/modules/cooler/README.md +++ b/modules/cooler/README.md @@ -175,7 +175,7 @@ func AllowExport(ctx context.Context, app *backpack.App, fallback cooler.Attempt | `cooler.SiteCachePurger` | Sends the site-cache purge request; a nil purger does nothing. | | `cooler.NewSiteCachePurger` | Builds the purger from the values of `site_cache.purge_url` and `site_cache.purge_token`; both empty returns nil; errors name those keys and never the token. | | `cooler.NewSiteCachePurgerFor` | The same with the config keys its errors name passed in, for a plugin that reads deprecated keys of its own. | -| `cooler.SiteCachePurger.Purge` | Sends one purge request (2 s timeout, detached from cancellation) and logs the outcome; never fails. | +| `cooler.SiteCachePurger.Purge` | Sends one purge request (2 s timeout, detached from cancellation, redirects not followed) and logs the outcome; never fails. | | `cooler.SiteCachePurger.Endpoint` | The purge URL's host and path, for log lines. | | `cooler.PurgeSiteCacheAfterCommit` | Registers one purge to run after the given transaction commits (at once outside a transaction). | | `cooler.SiteCacheFromApp` | Returns the published purger, or builds it from the framework keys and publishes it; nil when unset or invalid. | diff --git a/modules/cooler/sitecache.go b/modules/cooler/sitecache.go index 28d794b..517458f 100644 --- a/modules/cooler/sitecache.go +++ b/modules/cooler/sitecache.go @@ -90,9 +90,14 @@ func NewSiteCachePurgerFor(urlKey, tokenKey, purgeURL, token string) (*SiteCache if len(token) < SiteCachePurgeMinTokenLength { return nil, fmt.Errorf("%s must hold at least %d characters", tokenKey, SiteCachePurgeMinTokenLength) } - return &SiteCachePurger{url: u.String(), endpoint: u.Host + u.Path, token: token, client: &http.Client{}}, nil + return &SiteCachePurger{url: u.String(), endpoint: u.Host + u.Path, token: token, client: &http.Client{CheckRedirect: noRedirect}}, nil } +// noRedirect keeps the purge client on the configured URL: a 3xx answer is +// returned as is (and logged as a failed purge with its status), so the +// bearer token never follows a Location and the POST never becomes a GET. +func noRedirect(*http.Request, []*http.Request) error { return http.ErrUseLastResponse } + // Endpoint returns the purge URL's host and path, without the scheme, user // info or query, for log lines. func (p *SiteCachePurger) Endpoint() string { @@ -118,8 +123,9 @@ type siteCachePurgeBody struct { // Purge sends the purge request for one saved entity: a POST of // {"kind":kind,"slug":slug} with the bearer token, bounded by // SiteCachePurgeTimeout and detached from ctx's cancellation. It logs the -// outcome and returns nothing: a purge never fails a save. The token is -// never logged. +// outcome and returns nothing: a purge never fails a save. A redirect is +// never followed; a 3xx answer is logged as a failed purge with its status. +// The token is never logged. func (p *SiteCachePurger) Purge(ctx context.Context, kind, slug string) { if p == nil { return diff --git a/modules/cooler/sitecache_test.go b/modules/cooler/sitecache_test.go index b1de2b3..ce94928 100644 --- a/modules/cooler/sitecache_test.go +++ b/modules/cooler/sitecache_test.go @@ -639,3 +639,24 @@ func TestSiteCacheFromApp(t *testing.T) { t.Fatal("SiteCacheFromApp rebuilt a published purger") } } + +func TestSiteCachePurgeNoRedirect(t *testing.T) { + logs := captureLogs(t) + target, targetHits := purgeReceiver(t, func(w http.ResponseWriter, _ *http.Request) { _, _ = w.Write([]byte(`{"purged":1}`)) }) + srv, hits := purgeReceiver(t, func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Location", target.URL+"/api/__site__/cache/purge") + w.WriteHeader(http.StatusTemporaryRedirect) + }) + newTestPurger(t, srv).Purge(context.Background(), "post", "moved") + if len(hits()) != 1 { + t.Fatalf("%d requests to the purge URL, want 1", len(hits())) + } + if got := targetHits(); len(got) != 0 { + t.Fatalf("the redirect target received %d requests: %#v", len(got), got) + } + lines := logs.lines() + if len(lines) != 1 || !strings.Contains(lines[0], "level=WARN") || !strings.Contains(lines[0], `msg="cooler: site cache purge failed"`) || !strings.Contains(lines[0], "status=307") { + t.Fatalf("log = %q, want one failed purge with status=307", lines) + } + assertNoToken(t, logs) +}