From e6796fe9492bd542452be88aba9b62893ef03050 Mon Sep 17 00:00:00 2001 From: Jason Ernst Date: Fri, 2 Oct 2026 23:50:44 -0700 Subject: [PATCH 1/2] Stop re-escaping post slugs on every save The admin sends the stored, already escaped slug back on each save, and UpdatePost escaped it again: a colon became %3A on create, %253A after one edit, %25253A after two. The canonical URL is built from the stored slug, so every edit of such a post moved its canonical URL. - safeSlug unescapes fully before escaping, so saving is idempotent. - Post lookups match the canonical slug exactly instead of with LIKE, where the slug's own % escapes acted as wildcards and a URL could find a different post. Lookups canonicalize the requested slug, so the over-escaped URLs already in circulation still resolve. - A migration rewrites over-escaped slugs to the once-escaped form without touching UpdatedAt. Co-Authored-By: Claude Fable 5.1 --- admin/admin.go | 3 +++ admin/admin_test.go | 34 +++++++++++++++++++++++++++++ blog/blog.go | 15 ++++++++----- blog/blog_test.go | 51 +++++++++++++++++++++++++++++++++++++++++++ blog/post.go | 20 +++++++++++++++++ tools/migrate.go | 21 ++++++++++++++++++ tools/migrate_test.go | 43 ++++++++++++++++++++++++++++++++++++ 7 files changed, 181 insertions(+), 6 deletions(-) diff --git a/admin/admin.go b/admin/admin.go index a0181ab..e6ebdfc 100644 --- a/admin/admin.go +++ b/admin/admin.go @@ -120,7 +120,10 @@ func (a *Admin) UpdateDb(db *gorm.DB) { } // ////JSON API/////// +// safeSlug is idempotent: the admin sends back the stored, already escaped +// slug on every save, and escaping that again grew it by a layer each time. func safeSlug(slug string) string { + slug = blog.UnescapeSlug(slug) slug = strings.ReplaceAll(slug, " ", "-") slug = strings.ReplaceAll(slug, "/", "") slug = strings.ReplaceAll(slug, ".", "-") diff --git a/admin/admin_test.go b/admin/admin_test.go index 5d341c2..d0c4d05 100644 --- a/admin/admin_test.go +++ b/admin/admin_test.go @@ -1597,3 +1597,37 @@ func TestAdminAPI_StillAnswersJSON(t *testing.T) { } } } + +// TestUpdatePostKeepsSlug: the admin sends the stored slug back on every +// save; saving must not escape it again (%3A grew to %253A, %25253A, ...), +// and an already over-escaped slug comes back down to one layer. +func TestUpdatePostKeepsSlug(t *testing.T) { + db, _ := gorm.Open(sqlite.Open(":memory:")) + db.AutoMigrate(&auth.BlogUser{}, &blog.PostType{}, &blog.Post{}, &blog.Tag{}, &blog.Comment{}, &blog.Page{}, &blog.PostRevision{}) + pt := blog.PostType{Name: "Post", Slug: "posts"} + db.Create(&pt) + post := blog.Post{Title: "A: b", Slug: "A%3A-b", Content: "hi", PostTypeID: pt.ID} + db.Create(&post) + a := &Auth{} + b := blog.New(db, a, "test") + ad := admin.New(db, a, &b, "test") + router := gin.New() + router.PATCH("/api/v1/posts", ad.UpdatePost) + + for _, sent := range []string{"A%3A-b", "A%3A-b", "A%25253A-b"} { + body, _ := json.Marshal(blog.Post{ID: post.ID, Title: "A: b", Slug: sent, Content: "hi again", PostTypeID: pt.ID}) + a.On("IsAdmin", mock.Anything).Return(true).Once() + w := httptest.NewRecorder() + req, _ := http.NewRequest("PATCH", "/api/v1/posts", bytes.NewBuffer(body)) + req.Header.Add("Content-Type", "application/json") + router.ServeHTTP(w, req) + if w.Code != http.StatusAccepted { + t.Fatalf("PATCH slug %q: status %d: %s", sent, w.Code, w.Body.String()) + } + var got blog.Post + db.First(&got, post.ID) + if got.Slug != "A%3A-b" { + t.Fatalf("after saving slug %q the stored slug is %q, want A%%3A-b", sent, got.Slug) + } + } +} diff --git a/blog/blog.go b/blog/blog.go index 09b11ac..ac453cb 100644 --- a/blog/blog.go +++ b/blog/blog.go @@ -311,11 +311,11 @@ func (b *Blog) GetPostObject(c *gin.Context) (*Post, error) { return nil, errors.New("day must be an integer") } slug := c.Param("slug") - slug = url.QueryEscape(slug) + slug = CanonicalSlug(slug) log.Println("Looking for post: ", year, "/", month, "/", day, "/", slug) - if err := (*b.db).Preload("Tags").Preload("PostType").Where("created_at > ? AND slug LIKE ?", time.Date(year, time.Month(month), day, 0, 0, 0, 0, time.UTC), slug).First(&post).Error; err != nil { + if err := (*b.db).Preload("Tags").Preload("PostType").Where("created_at > ? AND LOWER(slug) = LOWER(?)", time.Date(year, time.Month(month), day, 0, 0, 0, 0, time.UTC), slug).First(&post).Error; err != nil { return nil, errors.New("No post at " + strconv.Itoa(year) + "/" + strconv.Itoa(month) + "/" + strconv.Itoa(day) + "/" + slug) } @@ -324,11 +324,14 @@ func (b *Blog) GetPostObject(c *gin.Context) (*Post, error) { return &post, nil } +// Posts are matched on the canonical slug, exactly: with LIKE the slug's +// own %XX escapes were wildcards. LOWER keeps the match case-insensitive +// on every database, as sqlite's LIKE was. func (b *Blog) getPostByParams(year int, month int, day int, slug string) (*Post, error) { log.Println("trying: " + strconv.Itoa(year) + "/" + strconv.Itoa(month) + "/" + strconv.Itoa(day) + "/" + slug) var post Post - slug = url.QueryEscape(slug) - if err := (*b.db).Preload("Tags").Preload("PostType").Where("created_at > ? AND slug LIKE ?", time.Date(year, time.Month(month), day, 0, 0, 0, 0, time.UTC), slug).First(&post).Error; err != nil { + slug = CanonicalSlug(slug) + if err := (*b.db).Preload("Tags").Preload("PostType").Where("created_at > ? AND LOWER(slug) = LOWER(?)", time.Date(year, time.Month(month), day, 0, 0, 0, 0, time.UTC), slug).First(&post).Error; err != nil { log.Println("NOT FOUND") return nil, errors.New("No post at " + strconv.Itoa(year) + "/" + strconv.Itoa(month) + "/" + strconv.Itoa(day) + "/" + slug) } @@ -627,9 +630,9 @@ func (b *Blog) getPostByTypeAndParams(typeSlug string, year int, month int, day return nil, err } var post Post - slug = url.QueryEscape(slug) + slug = CanonicalSlug(slug) if err := (*b.db).Preload("Tags").Preload("PostType"). - Where("post_type_id = ? AND created_at > ? AND slug LIKE ?", pt.ID, time.Date(year, time.Month(month), day, 0, 0, 0, 0, time.UTC), slug). + Where("post_type_id = ? AND created_at > ? AND LOWER(slug) = LOWER(?)", pt.ID, time.Date(year, time.Month(month), day, 0, 0, 0, 0, time.UTC), slug). First(&post).Error; err != nil { return nil, errors.New("No post at " + typeSlug + "/" + strconv.Itoa(year) + "/" + strconv.Itoa(month) + "/" + strconv.Itoa(day) + "/" + slug) } diff --git a/blog/blog_test.go b/blog/blog_test.go index 3f3f638..e7afb74 100644 --- a/blog/blog_test.go +++ b/blog/blog_test.go @@ -2068,3 +2068,54 @@ func TestCustomPage_ServerRendered(t *testing.T) { } } } + +// TestCanonicalSlug: a slug is escaped exactly once, however many layers +// of escaping it arrives with, and a lone % is not mistaken for an escape. +func TestCanonicalSlug(t *testing.T) { + for in, want := range map[string]string{ + "hello": "hello", + "A:-b": "A%3A-b", + "A%3A-b": "A%3A-b", + "A%253A-b": "A%3A-b", + "A%25253A-b": "A%3A-b", + "100%-done": "100%25-done", + "100%25-done": "100%25-done", + "100%2525-done": "100%25-done", + } { + if got := blog.CanonicalSlug(in); got != want { + t.Errorf("CanonicalSlug(%q) = %q, want %q", in, got, want) + } + } +} + +// TestPostLookupBySlug: a post answers at its canonical URL and at the +// over-escaped URLs earlier versions produced, in any case — and a % in +// the URL is not a wildcard that finds some other post. +func TestPostLookupBySlug(t *testing.T) { + db, _ := gorm.Open(sqlite.Open(":memory:")) + db.AutoMigrate(&auth.BlogUser{}, &blog.PostType{}, &blog.Post{}, &blog.Tag{}, &blog.Comment{}, &blog.Page{}, &blog.Setting{}) + pt := blog.PostType{Name: "Post", Slug: "posts"} + db.Create(&pt) + day := time.Date(2026, 8, 15, 12, 0, 0, 0, time.UTC) + db.Create(&blog.Post{Title: "A: b", Slug: "A%3A-b", Content: "hi", PostTypeID: pt.ID, CreatedAt: day}) + db.Create(&blog.Post{Title: "foo 25 bar", Slug: "foo-25-bar", Content: "hi", PostTypeID: pt.ID, CreatedAt: day}) + b := blog.New(db, &Auth{}, "test") + router := gin.New() + router.GET("/api/v1/posts/:yyyy/:mm/:dd/:slug", b.GetPost) + + for path, want := range map[string]int{ + "/api/v1/posts/2026/08/15/A%3A-b": http.StatusOK, + "/api/v1/posts/2026/08/15/A%253A-b": http.StatusOK, + "/api/v1/posts/2026/08/15/A%25253A-b": http.StatusOK, + "/api/v1/posts/2026/08/15/a%3a-B": http.StatusOK, + "/api/v1/posts/2026/08/15/foo-25-bar": http.StatusOK, + "/api/v1/posts/2026/08/15/foo%25-bar": http.StatusBadRequest, // GetPost's not-found; LIKE read this as foo25-bar + } { + w := httptest.NewRecorder() + req, _ := http.NewRequest("GET", path, nil) + router.ServeHTTP(w, req) + if w.Code != want { + t.Errorf("GET %s = %d, want %d", path, w.Code, want) + } + } +} diff --git a/blog/post.go b/blog/post.go index 20d450b..bf147c5 100644 --- a/blog/post.go +++ b/blog/post.go @@ -3,6 +3,7 @@ package blog import ( "bytes" "html/template" + "net/url" "regexp" "strings" "time" @@ -160,6 +161,25 @@ func (p Post) HTMLPreview(length int) template.HTML { return template.HTML(s) } +// UnescapeSlug undoes every layer of URL escaping on a slug, so one that +// has been escaped more than once (%253A) comes back as plain text (:). +func UnescapeSlug(slug string) string { + for { + plain, err := url.PathUnescape(slug) + if err != nil || plain == slug { + return slug + } + slug = plain + } +} + +// CanonicalSlug is the one stored form of a slug: escaped exactly once, +// however many times the input already was. Posts are looked up by it, so +// a URL that carries extra layers still finds its post. +func CanonicalSlug(slug string) string { + return url.QueryEscape(UnescapeSlug(slug)) +} + // Permalink returns the link to the post relative to root func (p Post) Permalink() string { typeSlug := "posts" diff --git a/tools/migrate.go b/tools/migrate.go index 5a93e5f..918de76 100644 --- a/tools/migrate.go +++ b/tools/migrate.go @@ -444,6 +444,7 @@ func Migrate(db *gorm.DB) error { seedDefaultPages(db) linkWritingPagesToPostType(db) cleanupEmptyTags(db) + repairOverEscapedSlugs(db) cleanupSelfExternalBacklinks(db) migrateSocialURLsToPlugin(db) cleanupPluginSettingsFromMainTable(db) @@ -562,6 +563,26 @@ func cleanupEmptyTags(db *gorm.DB) { } } +// repairOverEscapedSlugs rewrites post slugs that earlier versions escaped +// again on every save (%3A became %253A, then %25253A) to the canonical, +// once-escaped form. The old URLs still resolve: lookups canonicalize too. +func repairOverEscapedSlugs(db *gorm.DB) { + var posts []blog.Post + db.Select("id", "slug").Find(&posts) + for _, post := range posts { + fixed := blog.CanonicalSlug(post.Slug) + if fixed == post.Slug { + continue + } + // UpdateColumn: the post's content did not change, so neither does UpdatedAt. + if err := db.Model(&blog.Post{}).Where("id = ?", post.ID).UpdateColumn("slug", fixed).Error; err != nil { + log.Printf("Warning: failed to repair slug of post %d: %v", post.ID, err) + continue + } + log.Printf("Repaired slug of post %d: %s -> %s", post.ID, post.Slug, fixed) + } +} + // seedDefaultPages creates the default pages (Writing, About) if no pages exist. func seedDefaultPages(db *gorm.DB) { var count int64 diff --git a/tools/migrate_test.go b/tools/migrate_test.go index 242296b..54742bd 100644 --- a/tools/migrate_test.go +++ b/tools/migrate_test.go @@ -8,6 +8,7 @@ import ( "os" "strings" "testing" + "time" ) func TestMigration(t *testing.T) { @@ -348,3 +349,45 @@ func TestMigrationPrunesBlankUsers(t *testing.T) { } } } + +// TestMigrationRepairsOverEscapedSlugs: slugs that earlier versions escaped +// again on every save come back to one layer; healthy slugs and the posts' +// UpdatedAt are left alone. +func TestMigrationRepairsOverEscapedSlugs(t *testing.T) { + db, err := gorm.Open(sqlite.Open(":memory:"), &gorm.Config{ + DisableForeignKeyConstraintWhenMigrating: true, + }) + if err != nil { + t.Fatalf("failed to open sqlite db: %v", err) + } + if err := tools.Migrate(db); err != nil { + t.Fatalf("migration failed: %v", err) + } + updated := time.Date(2026, 8, 15, 12, 0, 0, 0, time.UTC) + slugs := map[string]string{ + "A%25253A-b": "A%3A-b", + "C%253A-d": "C%3A-d", + "E%3A-f": "E%3A-f", + "plain.slug": "plain.slug", + } + ids := map[string]uint{} + for stored := range slugs { + p := blog.Post{Title: stored, Slug: stored, Content: "hi", PostTypeID: 1, UpdatedAt: updated} + db.Create(&p) + ids[stored] = p.ID + } + + if err := tools.Migrate(db); err != nil { + t.Fatalf("second migration failed: %v", err) + } + for stored, want := range slugs { + var got blog.Post + db.First(&got, ids[stored]) + if got.Slug != want { + t.Errorf("slug %q became %q, want %q", stored, got.Slug, want) + } + if !got.UpdatedAt.Equal(updated) { + t.Errorf("slug %q: UpdatedAt changed to %v", stored, got.UpdatedAt) + } + } +} From 7bfad805a66871e2986be95aaaf563d32aded51d Mon Sep 17 00:00:00 2001 From: Jason Ernst Date: Sat, 3 Oct 2026 00:16:41 -0700 Subject: [PATCH 2/2] Log a failed post load in the slug repair migration Co-Authored-By: Claude Fable 5.1 --- tools/migrate.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tools/migrate.go b/tools/migrate.go index 918de76..b500d7c 100644 --- a/tools/migrate.go +++ b/tools/migrate.go @@ -568,7 +568,10 @@ func cleanupEmptyTags(db *gorm.DB) { // once-escaped form. The old URLs still resolve: lookups canonicalize too. func repairOverEscapedSlugs(db *gorm.DB) { var posts []blog.Post - db.Select("id", "slug").Find(&posts) + if err := db.Select("id", "slug").Find(&posts).Error; err != nil { + log.Printf("Warning: failed to load posts for slug repair: %v", err) + return + } for _, post := range posts { fixed := blog.CanonicalSlug(post.Slug) if fixed == post.Slug {