From 2d81c261b5cd9808d0d3df9d927a3f4f70d7da2f Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Tue, 27 Oct 2026 15:20:19 +0100 Subject: [PATCH] fix: exact redirects with a trailing slash or an upper-case query fire at runtime (CR-03) The resolver fetched exact rules with lower(source) IN keys, while the keys drop one trailing slash and keep the query as written, so a source stored as /old/ or /list?Page=2 was never read. A new migration adds the stored generated column source_key (the SQL form of exactKey) with an index for exact rules, and the fetch compares it. --- classes/redirects.go | 11 +++-- redirects_test.go | 43 +++++++++++++++++++ ...01_add_golem15_seo_redirects_source_key.go | 42 ++++++++++++++++++ 3 files changed, 92 insertions(+), 4 deletions(-) create mode 100644 updates/202610280001_add_golem15_seo_redirects_source_key.go diff --git a/classes/redirects.go b/classes/redirects.go index 83bccd8..4c6c66e 100644 --- a/classes/redirects.go +++ b/classes/redirects.go @@ -117,7 +117,9 @@ func toggleSlash(p string) string { } // exactKey is the lookup key of an exact source: lowercased, without a -// trailing slash on its path, with its query as written. +// trailing slash on its path, with its query as written. The column +// golem15_seo_redirects.source_key computes the same key in SQL (migration +// 202610280001); change both together. func exactKey(source string) string { p, q, hasQuery := strings.Cut(source, "?") key := strings.ToLower(trimSlash(p)) @@ -225,8 +227,9 @@ func LoadRedirects(db *gorm.DB) ([]models.Redirect, error) { } // LoadRedirectSet reads the rule set for the resolver: every pattern and -// regex rule now, exact rules per path as the set needs them (indexed on -// lower(source)). +// regex rule now, exact rules per path as the set needs them, by the +// generated source_key column, which holds exactKey(source) (so the fetch +// sees the same rules as an in-memory set). func LoadRedirectSet(db *gorm.DB) (*RedirectSet, error) { var rows []models.Redirect if err := db.Where("match <> ?", models.RedirectExact).Order("id ASC").Find(&rows).Error; err != nil { @@ -235,7 +238,7 @@ func LoadRedirectSet(db *gorm.DB) (*RedirectSet, error) { s := NewRedirectSet(rows) s.fetch = func(keys []string) ([]models.Redirect, error) { var out []models.Redirect - err := db.Where("match = ? AND lower(source) IN ?", models.RedirectExact, keys).Order("id ASC").Find(&out).Error + err := db.Where("match = ? AND source_key IN ?", models.RedirectExact, keys).Order("id ASC").Find(&out).Error return out, err } return s, nil diff --git a/redirects_test.go b/redirects_test.go index 0eca876..540b371 100644 --- a/redirects_test.go +++ b/redirects_test.go @@ -331,6 +331,49 @@ func TestRedirectEngine(t *testing.T) { } } +// TestExactRedirectKeys checks that the resolver's on-demand fetch of +// exact rules finds every source the in-memory matcher accepts (CR-03): a +// source stored with a trailing slash fires for both forms of the path, and +// one with upper-case letters in its query fires for that query. +func TestExactRedirectKeys(t *testing.T) { + env := bootEnv(t) + env.mustRedirect(t, "/old-page/", "exact", "/new-page", 301) + env.mustRedirect(t, "/list?Page=2", "exact", "/list-2", 301) + env.mustRedirect(t, "/Mixed/Case/", "exact", "/mixed-new", 301, func(r *models.Redirect) { r.IgnoreCase = true }) + + env.expectRedirect(t, "/old-page", 301, siteURL+"/new-page") + env.expectRedirect(t, "/old-page/", 301, siteURL+"/new-page") + env.expectRedirect(t, "/list?Page=2", 301, siteURL+"/list-2") + env.expectAction(t, "/list?page=2", "not_found") + env.expectRedirect(t, "/mixed/case", 301, siteURL+"/mixed-new") + + // The generated column holds the same key as classes.exactKey. + for source, want := range map[string]string{ + "/old-page/": "/old-page", + "/list?Page=2": "/list?Page=2", + "/Mixed/Case/": "/mixed/case", + } { + var key string + if err := env.db.Raw(`SELECT source_key FROM golem15_seo_redirects WHERE source = ?`, source).Scan(&key).Error; err != nil { + t.Fatal(err) + } + if key != want { + t.Fatalf("source_key of %q = %q, want %q", source, key, want) + } + } + env.seedRedirect(t, "/", "exact", "/home", 301) + env.seedRedirect(t, "/A/?Q=1", "exact", "/b", 301) + for source, want := range map[string]string{"/": "/", "/A/?Q=1": "/a?Q=1"} { + var key string + if err := env.db.Raw(`SELECT source_key FROM golem15_seo_redirects WHERE source = ?`, source).Scan(&key).Error; err != nil { + t.Fatal(err) + } + if key != want { + t.Fatalf("source_key of %q = %q, want %q", source, key, want) + } + } +} + // redirectCount is the number of stored redirects. func (e *testEnv) redirectCount(t *testing.T) int64 { t.Helper() diff --git a/updates/202610280001_add_golem15_seo_redirects_source_key.go b/updates/202610280001_add_golem15_seo_redirects_source_key.go new file mode 100644 index 0000000..faa8647 --- /dev/null +++ b/updates/202610280001_add_golem15_seo_redirects_source_key.go @@ -0,0 +1,42 @@ +package updates + +import ( + "github.com/go-gormigrate/gormigrate/v2" + "gorm.io/gorm" +) + +// redirectSourceKey is the SQL form of classes.exactKey: the source's path +// lowercased without one trailing slash (the root stays "/"), then its +// query as written. The resolver fetches exact rules by this key, so a +// source stored as "/old/" or "/list?Page=2" is found for every request the +// in-memory matcher accepts. +const redirectSourceKey = `(CASE + WHEN length(split_part(source, '?', 1)) > 1 AND right(split_part(source, '?', 1), 1) = '/' + THEN lower(left(split_part(source, '?', 1), -1)) + ELSE lower(split_part(source, '?', 1)) + END) || (CASE WHEN strpos(source, '?') > 0 THEN substr(source, strpos(source, '?')) ELSE '' END)` + +// addRedirectSourceKey adds golem15_seo_redirects.source_key, a stored +// generated column holding the exact-match lookup key, and indexes it for +// exact rules. Every write path fills it, because Postgres computes it. +var addRedirectSourceKey = []*gormigrate.Migration{ + { + ID: "202610280001_add_golem15_seo_redirects_source_key", + Migrate: func(tx *gorm.DB) error { + if err := tx.Exec(`ALTER TABLE golem15_seo_redirects ADD COLUMN source_key TEXT GENERATED ALWAYS AS (` + redirectSourceKey + `) STORED`).Error; err != nil { + return err + } + return tx.Exec(`CREATE INDEX golem15_seo_redirects_source_key ON golem15_seo_redirects (source_key) WHERE match = 'exact'`).Error + }, + Rollback: func(tx *gorm.DB) error { + if err := tx.Exec(`DROP INDEX IF EXISTS golem15_seo_redirects_source_key`).Error; err != nil { + return err + } + return tx.Exec(`ALTER TABLE golem15_seo_redirects DROP COLUMN IF EXISTS source_key`).Error + }, + }, +} + +func init() { + Register(addRedirectSourceKey...) +}