diff --git a/README.md b/README.md index 07e47a8..a462a29 100644 --- a/README.md +++ b/README.md @@ -92,7 +92,7 @@ location = /sitemap.xsl { The admin's type dropdown offers `url` and every registry type in registry order with its label. The reference dropdown depends on the row's type and lists the type's references (`signpost.ReferenceOptions`); `url` and types without references take none, and a reference the type does not offer is refused on save. -A save also refuses, per row, a `url` that is not a path (relative or starting with a single `/`) or an absolute `http`/`https` URL (`classes.AllowedItemURL`: no other scheme, no protocol-relative `//host`, no bare fragment, no surrounding whitespace or control characters), and a `urlPattern` that is not a path starting with a single `/` (`signpost.ValidItemPattern`). The errors are keyed `data..url` and `data..urlPattern`. +A save also refuses, per row, a missing `reference` for a registry type that takes one (such a row would resolve to nothing and be left out of the sitemap), a `url` that is not a path (relative or starting with a single `/`) or an absolute `http`/`https` URL (`classes.AllowedItemURL`: no other scheme, no protocol-relative `//host`, no bare fragment, no surrounding whitespace or control characters), and a `urlPattern` that is not a path starting with a single `/` (`signpost.ValidItemPattern`). The errors are keyed `data..reference`, `data..url` and `data..urlPattern`. ### Settings diff --git a/controllers/definitions.go b/controllers/definitions.go index 1587e9d..07e87f1 100644 --- a/controllers/definitions.go +++ b/controllers/definitions.go @@ -5,6 +5,7 @@ import ( "context" "errors" "fmt" + "strings" "git.golem15.com/golem15/sm-sitemap-plugin/classes" "git.golem15.com/golem15/sm-sitemap-plugin/models" @@ -30,6 +31,7 @@ const typeURLLabel = "golem15.sitemap::lang.item.url" const ( errURLScheme = "golem15.sitemap::lang.errors.url_scheme" errURLPattern = "golem15.sitemap::lang.errors.url_pattern" + errReference = "golem15.sitemap::lang.item.reference_required" ) // definitionsAdminController edits the one sitemap definition of the @@ -116,17 +118,25 @@ func (c definitionsAdminController) FormBeforeUpdate(_ context.Context, model an } // checkItems refuses, per row, a url outside classes.AllowedItemURL -// (data..url) and a URL pattern that is not a path -// (signpost.ValidItemPattern, data..urlPattern). The PHP plugin checked -// neither; a javascript: URL would reach , which sitemap.xsl renders as -// a link. -func (definitionsAdminController) checkItems(model any) error { +// (data..url), a URL pattern that is not a path +// (signpost.ValidItemPattern, data..urlPattern) and a missing reference +// for a registry type that takes one (data..reference). The PHP plugin +// checked none of them: a javascript: URL would reach , which +// sitemap.xsl renders as a link, and a row without its reference resolves to +// nothing and is left out of the sitemap without a word. +func (c definitionsAdminController) checkItems(model any) error { def, ok := model.(*models.Definition) if !ok || def == nil { return nil } + reg := c.registry() details := map[string]any{} for i, item := range def.Items() { + if item.Type != models.ItemTypeURL { + if t, known := reg.Type(item.Type); known && t.References != nil && strings.TrimSpace(models.Text(item.Reference)) == "" { + details[fmt.Sprintf("data.%d.reference", i)] = []string{errReference} + } + } if !classes.AllowedItemURL(models.Text(item.URL)) { details[fmt.Sprintf("data.%d.url", i)] = []string{errURLScheme} } diff --git a/sitemap_smoke_test.go b/sitemap_smoke_test.go index 2952365..79b9243 100644 --- a/sitemap_smoke_test.go +++ b/sitemap_smoke_test.go @@ -499,6 +499,39 @@ func TestSitemapAdminSmoke(t *testing.T) { t.Fatalf("PUT unknown reference: %d %s", rec.Code, rec.Body.String()) } + // A missing reference for a type that takes one (WR-03), a javascript: + // URL and a protocol-relative pattern (CR-01) are 422s on their rows. + for _, c := range []struct { + row int + key, value string + field string + message string + }{ + {1, "reference", "", "data.1.reference", "golem15.sitemap::lang.item.reference_required"}, + {1, "reference", " ", "data.1.reference", ""}, + {0, "url", "javascript://%0aalert(1)%0a//x", "data.0.url", "golem15.sitemap::lang.errors.url_scheme"}, + {1, "urlPattern", "//evil.test/:slug", "data.1.urlPattern", "golem15.sitemap::lang.errors.url_pattern"}, + } { + bad := []map[string]any{{}, {}} + for i := range rows { + for k, v := range rows[i] { + bad[i][k] = v + } + } + bad[1]["reference"] = "7" + bad[c.row][c.key] = c.value + rec = e.call(t, http.MethodPut, base+"/"+id, token, map[string]any{"data": bad}) + if rec.Code != http.StatusUnprocessableEntity || !strings.Contains(rec.Body.String(), `"`+c.field+`"`) { + t.Fatalf("PUT %s=%q: %d %s", c.key, c.value, rec.Code, rec.Body.String()) + } + if c.message != "" && !strings.Contains(rec.Body.String(), c.message) { + t.Fatalf("PUT %s=%q: want %s: %s", c.key, c.value, c.message, rec.Body.String()) + } + } + if err := e.gdb.Raw(`SELECT data FROM golem15_sitemap_definitions WHERE id = ?`, id).Scan(&raw).Error; err != nil || raw != wantData { + t.Fatalf("a refused save changed the data: %s %v", raw, err) + } + // The settings form saves the frontend URL. rec = e.call(t, http.MethodPut, "/settings/sitemap_settings", token, map[string]any{"frontend_url": "https://www.example.test", "enable_priority_tiers": true}) if rec.Code != http.StatusOK {