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.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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()
|
||||
|
||||
42
updates/202610280001_add_golem15_seo_redirects_source_key.go
Normal file
42
updates/202610280001_add_golem15_seo_redirects_source_key.go
Normal file
@@ -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...)
|
||||
}
|
||||
Reference in New Issue
Block a user