fix: one decoded path form for crawl keys and CMS lookups

Crawl keys, the rendered comparison and the link graph use the decoded
path (classes.PathKey), the form the resolver, the descriptors and the
SEO entries use; the request URL is percent-encoded again
(classes.EscapeKeyPath). A non-ASCII address now matches its CMS record
and its stored links. A path that does not decode to text keeps its
encoded form.

README.md

@@ -247,7 +247,7 @@ A crawl is a conga job (`golem15.seo.crawl`, one attempt) started from **Start c
 
 **What it fetches.** `robots.txt`, the sitemap index at `/sitemap.xml` and its files (or a single urlset), then breadth-first from the home page and the sitemap addresses over same-host links, redirect targets, canonicals and hreflang alternates. Requests go through a `fetchguard` client limited to the site host and its `www` twin: HTTPS only, private and reserved addresses refused at dial time, redirects never followed (each `Location` is a new address checked again), the User-Agent `SummerCMS-SEO-Crawler/1 (+<site>)`. Links to other hosts, credentials in a URL and plain http are never fetched. A staging or local site served over http or on a private address needs `golem15.seo.crawl_trusted: true` in config (or `SUMMER_GOLEM15__SEO__CRAWL_TRUSTED`), which no setting or MCP call can set; trusted mode still fetches only the site host and its twin. robots.txt rules are read for the report, not obeyed.
 
-**Limits.** Two requests at a time with the settings' pause between requests (default 250 ms), a 10 s timeout and a 5 MiB body cap per request, at most 1000 links read per page, and the settings' page limit (default 5000; a crawl that stops there is marked "limit reached"). Cancelling (**Cancel crawl**, `seo_cancel_crawl`) ends a queued crawl at once; a running crawl records the request (`cancel_requested` in the API and MCP answers, **Cancelling** in the list) and keeps running until its job stops between requests, then ends cancelled with what was fetched. Until then no other crawl starts (409 `crawls.already_running`) and the crawl cannot be deleted; a failed crawl keeps the pages and issues found before the failure and records the error. A crawl that reaches the job's time limit (`crawl.JobTimeout`, 6 h) ends failed with "the crawl reached its time limit of 6h" and keeps what it fetched: the final writes run on a context the job's cancellation cannot reach, bounded by `crawl.FinishTimeout` (10 min). A running crawl's job writes `heartbeat_at` every minute (`crawl.HeartbeatEvery`); a running crawl without a heartbeat for 10 minutes (`crawl.StaleAfter`, its job is gone, for example after a worker restart) is ended by the next start or delete (`crawl.RecoverStale`): failed with "the crawl job stopped without finishing (no heartbeat since …)", or cancelled when its cancel was requested. The crawler writes only its own tables (`golem15_seo_crawls`, `golem15_seo_crawl_pages`, `golem15_seo_crawl_links`, `golem15_seo_issues`); crawls beyond the settings' "Crawls kept" are deleted with their pages, links and issues.
+**Limits.** Two requests at a time with the settings' pause between requests (default 250 ms), a 10 s timeout and a 5 MiB body cap per request, at most 1000 links read per page, and the settings' page limit (default 5000; a crawl that stops there is marked "limit reached"). Cancelling (**Cancel crawl**, `seo_cancel_crawl`) ends a queued crawl at once; a running crawl records the request (`cancel_requested` in the API and MCP answers, **Cancelling** in the list) and keeps running until its job stops between requests, then ends cancelled with what was fetched. Until then no other crawl starts (409 `crawls.already_running`) and the crawl cannot be deleted; a failed crawl keeps the pages and issues found before the failure and records the error. A crawl that reaches the job's time limit (`crawl.JobTimeout`, 6 h) ends failed with "the crawl reached its time limit of 6h" and keeps what it fetched: the final writes run on a context the job's cancellation cannot reach, bounded by `crawl.FinishTimeout` (10 min). A running crawl's job writes `heartbeat_at` every minute (`crawl.HeartbeatEvery`); a running crawl without a heartbeat for 10 minutes (`crawl.StaleAfter`, its job is gone, for example after a worker restart) is ended by the next start or delete (`crawl.RecoverStale`): failed with "the crawl job stopped without finishing (no heartbeat since …)", or cancelled when its cancel was requested. Every crawled address is keyed by its decoded path with the raw query (`classes.PathKey`, the form of the resolver, the descriptors and the SEO entries; a path that does not decode to valid text, or holds `%` or `?`, keeps its encoded form) and requested percent-encoded (`classes.EscapeKeyPath`), so a non-ASCII address matches its CMS record, its focus keyphrase and its links in the E10 widget, written either way. The crawler writes only its own tables (`golem15_seo_crawls`, `golem15_seo_crawl_pages`, `golem15_seo_crawl_links`, `golem15_seo_issues`); crawls beyond the settings' "Crawls kept" are deleted with their pages, links and issues.
 
 **Checks** (kind, severity): `page_status` (error), `sitemap_error` (error), `sitemap_noindex` (error), `sitemap_blocked` (error), `sitemap_missing` (notice), `broken_link` (error, with the linking pages), `redirected_link` (notice), `canonical_missing` (warning), `canonical_error` (error), `missing_title` and `duplicate_title` (warning), `missing_description` and `duplicate_description` (notice), `h1_missing` and `h1_multiple` (error), `heading_skipped` and `heading_empty` (warning), `h1_title_mismatch` (notice), `jsonld_invalid` (error), `jsonld_multiple` (warning), `jsonld_required` (error) and `jsonld_recommended` (notice) from the same rules as the structured data preview (`schema.Validate`; FAQPage and other types without Google rules find nothing), `hreflang_reciprocal` (warning), `hreflang_error` (error), `generic_anchor` (warning, the settings' generic link texts, whole text, any case), `image_alt` (warning: an `<img>` without `alt`; `alt=""` is decorative), `image_size` and `featured_lazy` (notice), `content_js` (warning) and `orphan_page` (warning: an indexable sitemap page no crawled page links to, with up to 10 pages whose text mentions its title or focus keyphrase). The CMS-data checks run at the end of every crawl (and when the list is opened before any crawl) and are stored with the crawl's issues as source `cms`; a CMS issue the crawl found too is stored once.
 

classes/crawl/fetch.go

@@ -11,6 +11,7 @@ import (
 	"strings"
 	"time"
 
+	"git.golem15.com/golem15/sm-seo-plugin/classes"
 	"git.golem15.com/golem15/summercms/modules/compass"
 	"git.golem15.com/golem15/summercms/modules/fetchguard"
 )
@@ -168,25 +169,30 @@ func (f *Fetcher) Allowed(u *url.URL) bool {
 	return false
 }
 
-// Key is the address of u on the site: its path with the query, or ""
-// when u is not on the site (another host, scheme or port).
+// Key is the address of u on the site: its path in the decoded form of
+// classes.PathKey with the raw query, or "" when u is not on the site
+// (another host, scheme or port). The crawl's keys, the rendered
+// comparison, the CMS lookups and the link graph use this one form.
 func (f *Fetcher) Key(u *url.URL) string {
 	if !f.Allowed(u) {
 		return ""
 	}
-	p := u.EscapedPath()
-	if p == "" {
-		p = "/"
-	}
+	p := classes.PathKey(u)
 	if u.RawQuery != "" {
 		p += "?" + u.RawQuery
 	}
 	return p
 }
 
-// Absolute returns the site URL of an address key.
+// Absolute returns the site URL of an address key, its path
+// percent-encoded for the request (classes.EscapeKeyPath).
 func (f *Fetcher) Absolute(key string) string {
-	return f.site.Scheme + "://" + f.site.Host + key
+	p, q, hasQuery := strings.Cut(key, "?")
+	out := f.site.Scheme + "://" + f.site.Host + classes.EscapeKeyPath(p)
+	if hasQuery {
+		out += "?" + q
+	}
+	return out
 }
 
 // Get fetches one address key of the site.

classes/crawl/fetch_test.go

@@ -41,6 +41,32 @@ func TestFetcherPortAndKey(t *testing.T) {
 			t.Errorf("Allowed(%s) = %v", raw, f.Allowed(u))
 		}
 	}
+	// One decoded path form (WR-04, P33-61): the key is decoded, the
+	// request URL encoded; a path that does not decode to text keeps its
+	// encoded form, and so does one holding "%" or "?".
+	for raw, want := range map[string]string{
+		"https://site.test/blog/za%C5%BC%C3%B3%C5%82%C4%87?a=1": "/blog/zażółć?a=1",
+		"https://site.test/blog/zażółć":                         "/blog/zażółć",
+		"https://site.test/%FF":                                 "/%FF",
+		"https://site.test/a%00b":                               "/a%00b",
+		"https://site.test/100%25":                              "/100%25",
+		"https://site.test/a%3Fb?c=1":                           "/a%3Fb?c=1",
+	} {
+		if got := f.Key(parse(raw)); got != want {
+			t.Errorf("Key(%s) = %q, want %q", raw, got, want)
+		}
+	}
+	for key, want := range map[string]string{
+		"/blog/zażółć?a=1": "https://site.test/blog/za%C5%BC%C3%B3%C5%82%C4%87?a=1",
+		"/%FF":             "https://site.test/%FF",
+		"/100%25":          "https://site.test/100%25",
+		"/a b":             "https://site.test/a%20b",
+		"/":                "https://site.test/",
+	} {
+		if got := f.Absolute(key); got != want {
+			t.Errorf("Absolute(%q) = %q, want %q", key, got, want)
+		}
+	}
 	local, err := NewFetcher("http://localhost:3000", nil)
 	if err != nil {
 		t.Fatal(err)

classes/linkgraph.go

@@ -239,25 +239,51 @@ func (s *Service) LinkSuggestions(ctx context.Context, db *gorm.DB, path string,
 	return out, nil
 }
 
-// sitePath is the path of a path or an absolute URL; ErrBadURL otherwise.
+// sitePath is the crawl key of a path or an absolute URL (PathKey with
+// the raw query); ErrBadURL otherwise. A path and the absolute URL of the
+// same page give the same answer, written decoded or percent-encoded.
 func sitePath(raw string) (string, error) {
 	raw = strings.TrimSpace(raw)
 	if strings.HasPrefix(raw, "/") && !strings.HasPrefix(raw, "//") {
-		path, _, _ := strings.Cut(raw, "#")
-		return path, nil
+		u, err := url.Parse(raw)
+		if err != nil {
+			return "", ErrBadURL
+		}
+		return joinQuery(PathKey(u), u.RawQuery), nil
 	}
 	u, err := url.Parse(raw)
 	if err != nil || (u.Scheme != "http" && u.Scheme != "https") || u.Host == "" {
 		return "", ErrBadURL
 	}
-	path := u.EscapedPath()
-	if path == "" {
-		path = "/"
+	return joinQuery(PathKey(u), u.RawQuery), nil
+}
+
+// PathKey is the path form of the crawl's addresses (P33-61), the form
+// the resolver, the descriptors and the SEO entries use: u's decoded path
+// ("/" when empty). A path whose decoded form is not valid UTF-8 without
+// NUL, or holds "%" or "?" (which would read as an escape or a query),
+// keeps its percent-encoded form.
+func PathKey(u *url.URL) string {
+	if u == nil {
+		return "/"
 	}
-	if u.RawQuery != "" {
-		path += "?" + u.RawQuery
+	p := u.Path
+	if !safeText(p) || strings.ContainsAny(p, "%?") {
+		p = u.EscapedPath()
 	}
-	return path, nil
+	if p == "" {
+		p = "/"
+	}
+	return p
+}
+
+// EscapeKeyPath is the request form of a PathKey path: a decoded path is
+// percent-encoded, an encoded one (it holds "%") is kept.
+func EscapeKeyPath(p string) string {
+	if strings.Contains(p, "%") {
+		return p
+	}
+	return (&url.URL{Path: p}).EscapedPath()
 }
 
 // LinksOfURL is the internal links of a page addressed by URL (a path or

crawler_test.go

@@ -365,3 +365,79 @@ func TestStagingCrawl(t *testing.T) {
 	findIssue(t, list, "duplicate_title", "/a")
 	findIssue(t, list, "sitemap_missing", "/c")
 }
+
+// TestCrawlerNonASCII pins L3 WR-03 and WR-04 (P33-61) end to end: a
+// percent-encoded non-ASCII address is fetched once under its decoded key,
+// matches its CMS record (its content finding names the record type's
+// tool), its breadcrumb parent link is recognised (no false content_js
+// links finding), and the link graph reports its inbound link for the
+// encoded absolute URL; a link to the site host on another port is neither
+// fetched nor stored as a page.
+func TestCrawlerNonASCII(t *testing.T) {
+	env := bootCrawlEnv(t)
+	token := env.admin(t, true)
+	const (
+		post     = "/c/zażółć"
+		postEnc  = "/c/za%C5%BC%C3%B3%C5%82%C4%87"
+		cat      = "/cat/łąka"
+		catEnc   = "/cat/%C5%82%C4%85ka"
+		otherURL = "https://www.example.test:8443/x"
+	)
+	setCrawlRecords(t, map[string][]crawlRecord{
+		crawlPostType: {{id: "1", path: post, title: "Zażółć", headings: []pact.ContentHeading{{Level: 1, Text: "W treści"}},
+			parents: []pact.ContentRef{{Type: crawlCategoryType, ID: "2"}}}},
+		crawlCategoryType: {{id: "2", path: cat, title: "Łąka"}},
+	})
+	site := newFakeSite()
+	site.sitemap("/", postEnc)
+	site.plant(
+		doc{path: "/", title: "Home", links: []string{postEnc, catEnc, otherURL}},
+		doc{path: postEnc, title: "Zażółć", h1: []string{"Zażółć", "W treści"}, links: []string{catEnc}},
+		doc{path: catEnc, title: "Łąka", links: []string{"/"}},
+	)
+	id := env.crawlOnce(t, token, siteCtx(t, site))
+	if row := env.crawlRow(t, id); row.Status != "finished" {
+		t.Fatalf("crawl = %+v", row)
+	}
+
+	pages := env.storedPages(t, id)
+	if pages[post] != 200 || pages[cat] != 200 {
+		t.Fatalf("stored pages = %v, want %s and %s with 200", pages, post, cat)
+	}
+	for url := range pages {
+		if strings.Contains(url, "%C5") || url == "/x" {
+			t.Fatalf("stored page %q: an encoded key or the other port's page", url)
+		}
+	}
+	if site.requested(":8443") {
+		t.Fatal("the crawler fetched the site host on another port")
+	}
+	n := 0
+	for _, u := range site.requests {
+		if strings.Contains(u, postEnc) {
+			n++
+		}
+	}
+	if n != 1 {
+		t.Fatalf("%s was requested %d times, want once: %v", postEnc, n, site.requests)
+	}
+
+	list := env.issues(t, token, "")
+	content := findIssue(t, list, "h1_multiple", post)
+	if content.Fix != "tool" || content.Tool != "acme_fix_headings" {
+		t.Fatalf("the record's h1_multiple = %+v, want the record type's tool", content)
+	}
+	for _, i := range list {
+		if i.Kind == "content_js" && i.URL == post {
+			t.Fatalf("false content_js on the non-ASCII page: %+v", i)
+		}
+	}
+
+	report := env.links(t, token, "https://www.example.test"+postEnc)
+	if report.Inbound == nil || *report.Inbound != 1 {
+		t.Fatalf("inbound links of the encoded URL = %v, want 1", report.Inbound)
+	}
+	if decoded := env.links(t, token, post); decoded.Inbound == nil || *decoded.Inbound != 1 {
+		t.Fatalf("inbound links of the decoded path = %v, want 1", decoded.Inbound)
+	}
+}