fix: require ASCII digits for page numbers

docs/src/content/docs/reference/content-api.md

@@ -390,7 +390,8 @@ archive page. The front page is the exception: `/page/{n}` always
 answers the front page listing, and no item can be stored at
 `page/{n}`. Nothing published at an address answers
 `404 {"error":"content: not found"}`, and so does a page number below
-one. The resolver reads `/page/1` and `/page/02` as pages 1 and 2,
+one or holding a sign other than the digits 0 to 9, such as `/page/+2`.
+The resolver reads `/page/1` and `/page/02` as pages 1 and 2,
 while the built-in renderer redirects them to `/` and `/page/2`, as
 [what the binary serves where](/self-hosting/configuration/#what-the-binary-serves-where)
 describes.

internal/content/resolver.go

@@ -148,6 +148,9 @@ func listingAt(address string) (string, int, error) {
 	if !numbered {
 		return address, 1, nil
 	}
+	if strings.Trim(raw, "0123456789") != "" {
+		return "", 0, ErrNotFound
+	}
 	page, err := strconv.Atoi(raw)
 	if err != nil || page < 1 {
 		return "", 0, ErrNotFound

internal/content/resolver_test.go

@@ -158,6 +158,41 @@ func TestResolveLetsContentBeatPagination(t *testing.T) {
 	}
 }
 
+func TestResolveLetsContentBeatARefusedPageNumber(t *testing.T) {
+	t.Parallel()
+
+	for name, storedPath := range map[string]string{
+		"a word":             "page/about",
+		"a nested word":      "chapter/page/intro",
+		"an ordinal":         "page/2nd",
+		"zero":               "pages/page/0",
+		"negative":           "pages/page/-2",
+		"signed":             "pages/page/+2",
+		"a plus sign alone":  "pages/page/+",
+		"a minus sign alone": "pages/page/-",
+		"a fullwidth two":    "pages/page/\U0000FF12",
+		"a superscript two":  "pages/page/\U000000B2",
+		"too long to read":   "pages/page/99999999999999999999",
+	} {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+
+			store := newFakeAddresses()
+			held := store.hold("page", storedPath)
+			resolver := newResolver(t, store)
+
+			answer, err := resolver.Resolve(t.Context(), "/"+storedPath)
+			if err != nil {
+				t.Fatalf("Resolve error = %v, want nil", err)
+			}
+
+			if answer.Kind != content.KindItem || answer.Item.ID != held.ID {
+				t.Errorf("answer = %q %v, want the item stored at the address", answer.Kind, answer.Item.ID)
+			}
+		})
+	}
+}
+
 func TestResolveRefusesAPageNumberBelowTheFirst(t *testing.T) {
 	t.Parallel()
 
@@ -174,6 +209,40 @@ func TestResolveRefusesAPageNumberBelowTheFirst(t *testing.T) {
 	}
 }
 
+func TestResolveReadsAPageNumberOnlyFromPlainDigits(t *testing.T) {
+	t.Parallel()
+
+	resolver := newResolver(t, newFakeAddresses())
+
+	_, signed := resolver.Resolve(t.Context(), "/page/+2")
+	_, plusAlone := resolver.Resolve(t.Context(), "/page/+")
+	_, minusAlone := resolver.Resolve(t.Context(), "/page/-")
+	_, fullwidth := resolver.Resolve(t.Context(), "/page/\U0000FF12")
+	_, superscript := resolver.Resolve(t.Context(), "/page/\U000000B2")
+
+	for name, err := range map[string]error{
+		"signed": signed, "a plus sign alone": plusAlone, "a minus sign alone": minusAlone,
+		"a fullwidth two": fullwidth, "a superscript two": superscript,
+	} {
+		if !errors.Is(err, content.ErrNotFound) {
+			t.Errorf("%s page error = %v, want %v", name, err, content.ErrNotFound)
+		}
+	}
+}
+
+func TestResolveReadsEveryDigitOfAPageNumber(t *testing.T) {
+	t.Parallel()
+
+	answer, err := newResolver(t, newFakeAddresses()).Resolve(t.Context(), "/pages/page/1234567890")
+	if err != nil {
+		t.Fatalf("Resolve error = %v, want nil", err)
+	}
+
+	if answer.Kind != content.KindArchive || answer.Page != 1234567890 {
+		t.Errorf("answer = %q page %d, want the page archive at page 1234567890", answer.Kind, answer.Page)
+	}
+}
+
 func TestResolveReportsAnAddressHoldingNothing(t *testing.T) {
 	t.Parallel()
 

internal/publicsite/canonical_address_test.go

@@ -38,7 +38,6 @@ func TestSiteSendsAListingPageToTheOneAddressItAnswersAt(t *testing.T) {
 		{"/page/1", "/"},
 		{"/page/01", "/"},
 		{"/page/02", "/page/2"},
-		{"/page/+2", "/page/2"},
 		{"/page/1?utm_source=feed", "/?utm_source=feed"},
 		{"/page/02?utm_source=feed", "/page/2?utm_source=feed"},
 		{"/pages/page/1", "/pages"},
@@ -98,7 +97,6 @@ func TestSiteKeepsAListingPageWhoseOneAddressHoldsContent(t *testing.T) {
 		want      string
 	}{
 		{[]content.Content{two}, "/pages/page/02", "Pages | A Test Site"},
-		{[]content.Content{two}, "/pages/page/+2", "Pages | A Test Site"},
 		{[]content.Content{guide}, "/pages/page/1", "Pages | A Test Site"},
 		{[]content.Content{guide}, "/pages/page/01", "Pages | A Test Site"},
 		{[]content.Content{news, three}, "/categories/news/page/03", "News | A Test Site"},

internal/publicsite/page_number_test.go

@@ -0,0 +1,27 @@
+// SPDX-License-Identifier: Apache-2.0
+
+package publicsite_test
+
+import (
+	"net/http"
+	"testing"
+	"time"
+)
+
+func TestSiteReadsAPageNumberOnlyFromPlainDigits(t *testing.T) {
+	t.Parallel()
+
+	handler, _ := siteWith(
+		publishedPost("A Post", "a-post", blockMarkup, time.Now().UTC()),
+		storedAt("page", "Two", "pages/page/2"),
+		storedAt("category", "News", "categories/news"),
+	)
+
+	for _, address := range []string{"/page/+2", "/page/\U0000FF12", "/pages/page/+2", "/categories/news/page/+2"} {
+		recorder := get(t, handler, address)
+
+		if recorder.Code != http.StatusNotFound {
+			t.Errorf("status at %q = %d, want %d", address, recorder.Code, http.StatusNotFound)
+		}
+	}
+}

internal/server/content_values_test.go

@@ -938,6 +938,27 @@ func TestResolveKeepsASingleItemOffThePageWord(t *testing.T) {
 	}
 }
 
+func TestResolveReadsAPageNumberOnlyFromPlainDigits(t *testing.T) {
+	t.Parallel()
+
+	handler := termTypeServer(t)
+	publishItemAt(t, handler, storedCategoryItem(t, handler))
+
+	for path, want := range map[string]int{
+		"page/%2B2":                 http.StatusNotFound,
+		"categories/page/%2B2":      http.StatusNotFound,
+		"categories/news/page/%2B2": http.StatusNotFound,
+		"categories/page/02":        http.StatusOK,
+		"categories/news/page/02":   http.StatusOK,
+	} {
+		recorder := doRequest(t, handler, http.MethodGet, "/api/content/v1/resolve?path="+path, "")
+
+		if recorder.Code != want {
+			t.Errorf("resolve %s status = %d, want %d", path, recorder.Code, want)
+		}
+	}
+}
+
 func TestResolveCarriesRelationSummaries(t *testing.T) {
 	t.Parallel()
 

test/features/features/content-serving.feature

@@ -45,6 +45,11 @@ Feature: Serving content by address
     And the published page "2" filed under "Page"
     Then "/pages/page/2" answers with "2"
 
+  Scenario: Exact content beats a number that is no page number
+    Given the published page "Page"
+    And the published page "٢" filed under "Page"
+    Then "/pages/page/٢" answers with "٢"
+
   Scenario: A padded page number stays where content holds the plain number
     Given the published page "Page"
     And the published page "2" filed under "Page"
@@ -72,6 +77,10 @@ Feature: Serving content by address
   Scenario: A page number below the first answers not found
     Then "/page/0" answers not found
 
+  Scenario: A signed page number answers not found
+    Then "/page/+2" answers not found
+    And "/pages/page/+2" answers not found
+
   Scenario: An archive of an inactive type answers not found
     Given the type "page" is deactivated
     Then "/pages" answers not found