diff --git a/docs/src/content/docs/reference/content-api.md b/docs/src/content/docs/reference/content-api.md index 9972180d..9ba402a5 100644 --- a/docs/src/content/docs/reference/content-api.md +++ b/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. diff --git a/internal/content/resolver.go b/internal/content/resolver.go index a7f705ce..9095acef 100644 --- a/internal/content/resolver.go +++ b/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 diff --git a/internal/content/resolver_test.go b/internal/content/resolver_test.go index 39375ff0..298084af 100644 --- a/internal/content/resolver_test.go +++ b/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() diff --git a/internal/publicsite/canonical_address_test.go b/internal/publicsite/canonical_address_test.go index 9c051ca9..aa0a3a40 100644 --- a/internal/publicsite/canonical_address_test.go +++ b/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"}, diff --git a/internal/publicsite/page_number_test.go b/internal/publicsite/page_number_test.go new file mode 100644 index 00000000..f67e708d --- /dev/null +++ b/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) + } + } +} diff --git a/internal/server/content_values_test.go b/internal/server/content_values_test.go index 10cbd60b..3075bab6 100644 --- a/internal/server/content_values_test.go +++ b/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() diff --git a/test/features/features/content-serving.feature b/test/features/features/content-serving.feature index 773e5fb6..c6af87f4 100644 --- a/test/features/features/content-serving.feature +++ b/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