Fixed uniqueness validation of Recommendation URL (#18383)

refs https://github.com/TryGhost/Product/issues/3818

- added a check to compare hostname and pathnames of URLs. Different
subdomain or different pathname = different URLs, but protocol, www,
query parameters and hash fragments are ignored.

apps/admin-x-settings/src/components/settings/site/recommendations/AddRecommendationModal.tsx

@@ -10,9 +10,9 @@ import {AlreadyExistsError} from '../../../../utils/errors';
 import {EditOrAddRecommendation, RecommendationResponseType, useGetRecommendationByUrl} from '../../../../api/recommendations';
 import {LoadingIndicator} from '../../../../admin-x-ds/global/LoadingIndicator';
 import {RoutingModalProps} from '../../../providers/RoutingProvider';
+import {arePathsEqual, trimSearchAndHash} from '../../../../utils/url';
 import {dismissAllToasts, showToast} from '../../../../admin-x-ds/global/Toast';
 import {formatUrl} from '../../../../admin-x-ds/global/form/URLTextField';
-import {trimSearchAndHash} from '../../../../utils/url';
 import {useExternalGhostSite} from '../../../../api/external-ghost-site';
 import {useGetOembed} from '../../../../api/oembed';
 
@@ -59,7 +59,11 @@ const AddRecommendationModal: React.FC<RoutingModalProps & AddRecommendationModa
             // Check if the recommendation already exists
             const {recommendations = []} = await getRecommendationByUrl(validatedUrl) as RecommendationResponseType;
             if (recommendations && recommendations.length > 0) {
-                throw new AlreadyExistsError('A recommendation with this URL already exists.');
+                const existing = recommendations.find(r => arePathsEqual(r.url, validatedUrl.toString()));
+
+                if (existing) {
+                    throw new AlreadyExistsError('A recommendation with this URL already exists.');
+                }
             }
 
             // Check if it's a Ghost site or not:

apps/admin-x-settings/src/utils/url.ts

@@ -13,3 +13,26 @@ export function trimSearchAndHash(url: URL) {
     url.hash = '';
     return url;
 }
+
+/*  Compare two URLs based on their hostname and pathname.
+ *  Query params, hash fragements, protocol and www are ignored.
+ *
+ *  Example:
+ *  - https://a.com, http://a.com, https://www.a.com, https://a.com?param1=value, https://a.com/#segment-1 are all considered equal
+ *  - but, https://a.com/path-1 and  https://a.com/path-2 are not
+ */
+export function arePathsEqual(urlStr1: string, urlStr2: string) {
+    let url1, url2;
+
+    try {
+        url1 = new URL(urlStr1);
+        url2 = new URL(urlStr2);
+    } catch (e) {
+        return false;
+    }
+
+    return (
+        url1.hostname.replace('www.', '') === url2.hostname.replace('www.', '') &&
+        url1.pathname === url2.pathname
+    );
+}

apps/admin-x-settings/test/unit/utils/url.test.ts

@@ -1,5 +1,5 @@
 import * as assert from 'assert/strict';
-import {trimHash, trimSearch, trimSearchAndHash} from '../../../src/utils/url';
+import {arePathsEqual, trimHash, trimSearch, trimSearchAndHash} from '../../../src/utils/url';
 
 describe('trimSearch', function () {
     it('removes the query parameters from a URL', function () {
@@ -27,3 +27,68 @@ describe('trimSearchAndHash', function () {
         assert.equal(trimSearchAndHash(parsedUrl).toString(), 'https://example.com/path');
     });
 });
+
+describe('arePathsEqual', function () {
+    it('returns false if one of the param is not a URL', function () {
+        const url1 = 'foo';
+        const url2 = 'https://example.com';
+
+        assert.equal(arePathsEqual(url1, url2), false);
+    });
+
+    it('returns false if hostnames are different', function () {
+        const url1 = 'https://a.com';
+        const url2 = 'https://b.com';
+
+        assert.equal(arePathsEqual(url1, url2), false);
+    });
+
+    it('returns false if top level domains are different', function () {
+        const url1 = 'https://a.io';
+        const url2 = 'https://a.com';
+
+        assert.equal(arePathsEqual(url1, url2), false);
+    });
+
+    it('returns false if sub domains are different', function () {
+        const url1 = 'https://sub.a.com';
+        const url2 = 'https://subdiff.a.com';
+
+        assert.equal(arePathsEqual(url1, url2), false);
+    });
+
+    it('returns false if paths are different', function () {
+        const url1 = 'https://a.com/path-1';
+        const url2 = 'https://a.com/path-2';
+
+        assert.equal(arePathsEqual(url1, url2), false);
+    });
+
+    it('returns true even if protocols are different', function () {
+        const url1 = 'http://a.com';
+        const url2 = 'https://a.com';
+
+        assert.equal(arePathsEqual(url1, url2), true);
+    });
+
+    it('returns true even if www is used in one of the urls', function () {
+        const url1 = 'https://www.a.com';
+        const url2 = 'https://a.com';
+
+        assert.equal(arePathsEqual(url1, url2), true);
+    });
+
+    it('returns true even if query parameters are different', function () {
+        const url1 = 'http://a.com?foo=bar';
+        const url2 = 'http://a.com';
+
+        assert.equal(arePathsEqual(url1, url2), true);
+    });
+
+    it('returns true even if hash segments are different', function () {
+        const url1 = 'http://a.com#segment-1';
+        const url2 = 'http://a.com';
+
+        assert.equal(arePathsEqual(url1, url2), true);
+    });
+});

ghost/core/content/themes/casper

@@ -1 +1 @@
-Subproject commit 4d3319d05ce92e7b0244e5608d3fc6cc9c86e735
+Subproject commit 276e2c9d0140c902e1c8d3760bc194790722fa71

ghost/recommendations/src/BookshelfRecommendationRepository.ts

@@ -75,8 +75,21 @@ export class BookshelfRecommendationRepository extends BookshelfRepository<strin
         } as Record<keyof Recommendation, string>;
     }
 
-    async getByUrl(url: URL): Promise<Recommendation[]> {
+    async getByUrl(url: URL): Promise<Recommendation | null> {
         const urlFilter = `url:~'${url.host.replace('www.', '')}${url.pathname.replace(/\/$/, '')}'`;
-        return this.getPage({filter: urlFilter, page: 1, limit: 1});
+        const recommendations = await this.getAll({filter: urlFilter});
+
+        if (!recommendations || recommendations.length === 0) {
+            return null;
+        }
+
+        //  Find URL based on the hostname and pathname.
+        //  Query params, hash fragements, protocol and www are ignored.
+        const existing = recommendations.find((r) => {
+            return r.url.hostname.replace('www.', '') === url.hostname.replace('www.', '') &&
+                   r.url.pathname === url.pathname;
+        }) || null;
+
+        return existing;
     }
 }

ghost/recommendations/src/InMemoryRecommendationRepository.ts

@@ -7,8 +7,14 @@ export class InMemoryRecommendationRepository extends InMemoryRepository<string,
         return entity;
     }
 
-    getByUrl(url: URL): Promise<Recommendation[]> {
-        const urlFilter = `url:~'${url.host.replace('www.', '')}${url.pathname.replace(/\/$/, '')}'`;
-        return this.getPage({filter: urlFilter, page: 1, limit: 1});
+    async getByUrl(url: URL): Promise<Recommendation | null > {
+        //  Find URL based on the hostname and pathname.
+        //  Query params, hash fragements, protocol and www are ignored.
+        const existing = this.store.find((r) => {
+            return r.url.hostname.replace('www.', '') === url.hostname.replace('www.', '') &&
+                   r.url.pathname === url.pathname;
+        }) || null;
+
+        return existing;
     }
 }

ghost/recommendations/src/RecommendationRepository.ts

@@ -4,7 +4,7 @@ import {Recommendation} from './Recommendation';
 export interface RecommendationRepository {
     save(entity: Recommendation): Promise<void>;
     getById(id: string): Promise<Recommendation | null>;
-    getByUrl(url: URL): Promise<Recommendation[]>;
+    getByUrl(url: URL): Promise<Recommendation | null>;
     getAll({filter, order}?: {filter?: string, order?: OrderOption<Recommendation>}): Promise<Recommendation[]>;
     getPage({filter, order, page, limit}: {
         filter?: string;

ghost/recommendations/src/RecommendationService.ts

@@ -109,7 +109,7 @@ export class RecommendationService {
 
         // If a recommendation with this URL already exists, throw an error
         const existing = await this.repository.getByUrl(recommendation.url);
-        if (existing && existing.length > 0) {
+        if (existing) {
             throw new errors.ValidationError({
                 message: 'A recommendation with this URL already exists.'
             });