Added router-filter scalar columns to lazy URL field requirements

ref https://linear.app/ghost/issue/HKG-1875

A filtered collection router such as featured:true also needs its filter's own columns on the resource to choose the same router eager would. These were neither reported by getRequiredFields nor required on the forward path, so under ?fields=url a thin record let filterMatches see an undefined field and a featured post routed to /:slug/ instead of /featured/:slug/. The non-relation scalar fields a router filter references are now reported so the serializers force them, and required on the forward path so a thin resource is rejected rather than silently mis-routed. Field names are matched only at NQL expression boundaries, so colon-bearing values such as URLs or timestamps are not mistaken for fields; relation fields and the page/type discriminator stay excluded.

Author: allouis

ghost/core/core/server/services/url/lazy-url-service.ts

@@ -57,6 +57,38 @@ function buildBaseFilters(): Map<string, BaseFilter> {
     return baseFilters;
 }
 
+// Relation roots are loaded via getRequiredRelations (as withRelated), not as
+// scalar columns; `page`/`type` are the router-type discriminator, always set
+// on the resource. Everything else a router filter references is a scalar
+// own-column the resource must carry to be routed like eager.
+const FILTER_NON_SCALAR_FIELDS = new Set([
+    'tag', 'tags', 'author', 'authors', 'primary_tag', 'primary_author', 'page', 'type'
+]);
+
+// Scalar (own-column) fields a router filter reads, e.g. 'featured' from
+// 'featured:true'. Dotted clauses (e.g. tags.visibility) are relation
+// sub-fields and skipped — getRequiredRelations loads those relations.
+//
+// Only field names at an NQL expression boundary (start of the filter, or after
+// a `+`/`,`/`(` combinator) are matched, so colon-bearing values — URLs,
+// timestamps like 2020-01-01T00:00:00 — aren't mistaken for fields.
+function filterScalarFields(filter: string | null): string[] {
+    if (!filter) {
+        return [];
+    }
+    const fields = new Set<string>();
+    const matcher = /(?:^|[+,(])\s*(\w+)(\.\w+)?:/g;
+    let match;
+    while ((match = matcher.exec(filter)) !== null) {
+        const [, root, sub] = match;
+        if (sub || FILTER_NON_SCALAR_FIELDS.has(root)) {
+            continue;
+        }
+        fields.add(root);
+    }
+    return [...fields];
+}
+
 interface LazyUrlServiceDeps {
     urlUtils?: typeof localUtils;
     findResource: FindResource;
@@ -143,8 +175,8 @@ export class LazyUrlService implements LazyUrlServiceBackend {
 
     // Columns a resource of this type must carry for the lazy service to build
     // its URL: its base-filter columns plus the scalar columns its routers'
-    // permalinks substitute. Permalink relations are covered separately by
-    // getRequiredRelations; eager needs none of this (it looks URLs up by id).
+    // permalinks substitute and filters read. Relations are covered separately
+    // by getRequiredRelations; eager needs none of this (it looks URLs up by id).
     getRequiredFields(routerType: string): string[] {
         const fields = new Set<string>();
         const base = this.baseFilters.get(routerType);
@@ -164,6 +196,7 @@ export class LazyUrlService implements LazyUrlServiceBackend {
             if (/\b(year|month|day)\b/.test(config.permalink)) {
                 fields.add('published_at');
             }
+            filterScalarFields(config.filter).forEach(field => fields.add(field));
         }
         return [...fields];
     }
@@ -374,6 +407,11 @@ export class LazyUrlService implements LazyUrlServiceBackend {
         if (/\bprimary_author\b/.test(config.filter) && r.primary_author === undefined) {
             missing.push('primary_author');
         }
+        for (const field of filterScalarFields(config.filter)) {
+            if (r[field] === undefined) {
+                missing.push(field);
+            }
+        }
         if (missing.length === 0) {
             return;
         }

ghost/core/test/unit/server/services/url/lazy-url-service.test.js

@@ -132,6 +132,34 @@ describe('LazyUrlService', function () {
             );
         });
 
+        it('throws when a router filter references a scalar field the resource lacks (e.g. featured)', function () {
+            const service = new LazyUrlService({urlUtils, findResource: noopFindResource});
+            // featured:true needs the featured column; without it the filter would
+            // see undefined and silently route to the wrong permalink.
+            service.onRouterAddedType('featured', 'featured:true', 'posts', '/featured/:slug/');
+            service.onRouterAddedType('default', null, 'posts', '/:slug/');
+
+            assert.throws(
+                () => service.getUrlForResource({type: 'posts', id: 'p', slug: 'hot', status: 'published'}),
+                /Thin resource passed to LazyUrlService/
+            );
+        });
+
+        it('routes via the filtered router when its scalar field is present', function () {
+            const service = new LazyUrlService({urlUtils, findResource: noopFindResource});
+            service.onRouterAddedType('featured', 'featured:true', 'posts', '/featured/:slug/');
+            service.onRouterAddedType('default', null, 'posts', '/:slug/');
+
+            assert.equal(
+                service.getUrlForResource({type: 'posts', id: 'p', slug: 'hot', status: 'published', featured: true}),
+                '/featured/hot/'
+            );
+            assert.equal(
+                service.getUrlForResource({type: 'posts', id: 'p', slug: 'meh', status: 'published', featured: false}),
+                '/meh/'
+            );
+        });
+
         it('expands shorthand tag/author filters via the EXPANSIONS table', function () {
             const service = new LazyUrlService({urlUtils, findResource: noopFindResource});
             service.onRouterAddedType('podcast', 'tag:podcast', 'posts', '/podcast/:slug/');
@@ -685,5 +713,31 @@ describe('LazyUrlService', function () {
             assert.deepEqual(service.getRequiredFields('posts').sort(), ['published_at', 'slug', 'status', 'type']);
             assert.deepEqual(service.getRequiredFields('pages').sort(), ['id', 'status', 'type']);
         });
+
+        it('includes scalar columns referenced by a router filter, but not relation or discriminator fields', function () {
+            const service = new LazyUrlService({urlUtils, findResource: noopFindResource});
+            service.onRouterAddedType('featured', 'featured:true', 'posts', '/featured/:slug/');
+            service.onRouterAddedType('default', null, 'posts', '/:slug/');
+
+            // featured (scalar) is required; page/type (discriminator) and tag/
+            // author relations are not — those go through getRequiredRelations.
+            assert.deepEqual(service.getRequiredFields('posts').sort(), ['featured', 'slug', 'status', 'type']);
+        });
+
+        it('does not treat relation or page/type filter clauses as scalar columns', function () {
+            const service = new LazyUrlService({urlUtils, findResource: noopFindResource});
+            service.onRouterAddedType('news', 'tag:news+page:false', 'posts', '/:slug/');
+
+            assert.deepEqual(service.getRequiredFields('posts').sort(), ['slug', 'status', 'type']);
+        });
+
+        it('does not capture colon-bearing filter values (timestamps, URLs) as fields', function () {
+            const service = new LazyUrlService({urlUtils, findResource: noopFindResource});
+            // The timestamp value contains `00:00:00`; only `published_at` (at an
+            // expression boundary) is a field, not `00`.
+            service.onRouterAddedType('recent', "published_at:>'2020-01-01T00:00:00Z'", 'posts', '/:slug/');
+
+            assert.deepEqual(service.getRequiredFields('posts').sort(), ['published_at', 'slug', 'status', 'type']);
+        });
     });
 });