FIX: remove trailing slashes and query params on meta-tag-updater's canonical url (#24445)

* FIX: remove trailing slashes and query params on meta-tag-updater's canonical url

app/assets/javascripts/discourse/app/instance-initializers/meta-tag-updater.js

@@ -1,3 +1,4 @@
+import { getCanonicalUrl } from "discourse/lib/url";
 import { getAbsoluteURL } from "discourse-common/lib/get-url";
 
 export default {
@@ -21,7 +22,9 @@ export default {
       twitterTitle?.setAttribute("content", title);
       twitterUrl?.setAttribute("content", absoluteUrl);
 
-      canonicalUrl?.setAttribute("href", absoluteUrl);
+      if (canonicalUrl) {
+        canonicalUrl.setAttribute("href", getCanonicalUrl(absoluteUrl));
+      }
     });
   },
 };

app/assets/javascripts/discourse/app/lib/url.js

@@ -39,6 +39,9 @@ const SERVER_SIDE_ONLY = [
 // scrolling to "suggested topics".
 const JUMP_END_BUFFER = 250;
 
+const ALLOWED_CANONICAL_PARAMS = ["page"];
+const TRAILING_SLASH_REGEX = /\/$/;
+
 export function rewritePath(path) {
   const params = path.split("?");
 
@@ -506,4 +509,22 @@ export function getEditCategoryUrl(category, subcategories, tab) {
   return getURL(url);
 }
 
+export function getCanonicalUrl(absoluteUrl) {
+  const canonicalUrl = new URL(absoluteUrl);
+  canonicalUrl.pathname = canonicalUrl.pathname.replace(
+    TRAILING_SLASH_REGEX,
+    ""
+  );
+
+  const allowedSearchParams = new URLSearchParams();
+  for (const [key, value] of canonicalUrl.searchParams) {
+    if (ALLOWED_CANONICAL_PARAMS.includes(key)) {
+      allowedSearchParams.append(key, value);
+    }
+  }
+  canonicalUrl.search = allowedSearchParams.toString();
+
+  return canonicalUrl.toString();
+}
+
 export default _urlInstance;

app/assets/javascripts/discourse/tests/acceptance/admin-site-settings-test.js

@@ -148,7 +148,7 @@ acceptance("Admin - Site Settings", function (needs) {
   });
 
   test("category name is preserved", async function (assert) {
-    await visit("admin/site_settings/category/basic?filter=menu");
+    await visit("/admin/site_settings/category/basic?filter=menu");
     assert.strictEqual(
       currentURL(),
       "/admin/site_settings/category/basic?filter=menu"
@@ -156,7 +156,7 @@ acceptance("Admin - Site Settings", function (needs) {
   });
 
   test("shows all_results if current category has none", async function (assert) {
-    await visit("admin/site_settings");
+    await visit("/admin/site_settings");
 
     await click(".admin-nav .basic a");
     assert.strictEqual(currentURL(), "/admin/site_settings/category/basic");

app/assets/javascripts/discourse/tests/unit/lib/url-test.js

@@ -2,6 +2,7 @@ import { setupTest } from "ember-qunit";
 import { module, test } from "qunit";
 import sinon from "sinon";
 import DiscourseURL, {
+  getCanonicalUrl,
   getCategoryAndTagUrl,
   prefixProtocol,
   userPath,
@@ -178,4 +179,38 @@ module("Unit | Utility | url", function (hooks) {
       "in-page anchors call replaceState with the url fragment"
     );
   });
+
+  test("getCanonicalUrl", function (assert) {
+    assert.strictEqual(
+      getCanonicalUrl("http://eviltrout.com/t/this-is-a-test/1/"),
+      "http://eviltrout.com/t/this-is-a-test/1",
+      "trailing slashes are removed"
+    );
+
+    assert.strictEqual(
+      getCanonicalUrl(
+        "http://eviltrout.com/t/this-is-a-test/1/?page=2&u=john&not_allowed=true"
+      ),
+      "http://eviltrout.com/t/this-is-a-test/1?page=2",
+      "disallowed query params are removed"
+    );
+
+    assert.strictEqual(
+      getCanonicalUrl("http://eviltrout.com/t/this-is-a-test/2"),
+      "http://eviltrout.com/t/this-is-a-test/2",
+      "canonical urls are not modified"
+    );
+
+    assert.strictEqual(
+      getCanonicalUrl("http://eviltrout.com/t/this-is-a-test/2/?"),
+      "http://eviltrout.com/t/this-is-a-test/2",
+      "trailing /? are removed"
+    );
+
+    assert.strictEqual(
+      getCanonicalUrl("http://eviltrout.com/t/this-is-a-test/2?"),
+      "http://eviltrout.com/t/this-is-a-test/2",
+      "trailing ? are removed"
+    );
+  });
 });