From 8edc42aa7c209b12d98ecf20cdecccddf7314af0 Mon Sep 17 00:00:00 2001
From: Alex Nguyen <dev@alexnguyen.co.nz>
Date: Thu, 14 Mar 2024 05:54:09 +1300
Subject: [PATCH] prevent URLs in attributes being escaped (#9820)

* prevent URLs in `content` attributes being escapedk

* removed content check

* Update .changeset/quick-islands-ring.md

Co-authored-by: Florian Lefebvre <contact@florian-lefebvre.dev>

* add check for '&' in string

* Update .changeset/quick-islands-ring.md

* manual `canParse`

* add test

---------

Co-authored-by: Florian Lefebvre <contact@florian-lefebvre.dev>
Co-authored-by: Nate Moore <natemoo-re@users.noreply.github.com>
Co-authored-by: lilnasy <69170106+lilnasy@users.noreply.github.com>
---
 .changeset/quick-islands-ring.md                   |  5 +++++
 packages/astro/src/runtime/server/render/util.ts   | 14 ++++++++++++++
 packages/astro/test/astro-attrs.test.js            |  3 +++
 .../fixtures/astro-attrs/src/pages/index.astro     |  1 +
 4 files changed, 23 insertions(+)
 create mode 100644 .changeset/quick-islands-ring.md

diff --git a/.changeset/quick-islands-ring.md b/.changeset/quick-islands-ring.md
new file mode 100644
index 000000000000..a3b3e14ca04b
--- /dev/null
+++ b/.changeset/quick-islands-ring.md
@@ -0,0 +1,5 @@
+---
+"astro": patch
+---
+
+Prevents fully formed URLs in attributes from being escaped
diff --git a/packages/astro/src/runtime/server/render/util.ts b/packages/astro/src/runtime/server/render/util.ts
index 3fa01aeb616e..52149a0319f2 100644
--- a/packages/astro/src/runtime/server/render/util.ts
+++ b/packages/astro/src/runtime/server/render/util.ts
@@ -104,6 +104,11 @@ Make sure to use the static attribute syntax (\`${key}={value}\`) instead of the
 		return markHTMLString(` class="${toAttributeString(value, shouldEscape)}"`);
 	}
 
+	// Prevents URLs in attributes from being escaped in static builds
+	if (typeof value === 'string' && value.includes('&') && urlCanParse(value)) {
+		return markHTMLString(` ${key}="${toAttributeString(value, false)}"`);
+	}
+
 	// Boolean values only need the key
 	if (value === true && (key.startsWith('data-') || htmlBooleanAttributes.test(key))) {
 		return markHTMLString(` ${key}`);
@@ -224,3 +229,12 @@ export function promiseWithResolvers<T = any>(): PromiseWithResolvers<T> {
 		reject,
 	};
 }
+
+function urlCanParse(url: string) {
+	try {
+		new URL(url);
+		return true;
+	} catch {
+		return false;
+	}
+}
diff --git a/packages/astro/test/astro-attrs.test.js b/packages/astro/test/astro-attrs.test.js
index 0866e4828ba3..4ee547d91d56 100644
--- a/packages/astro/test/astro-attrs.test.js
+++ b/packages/astro/test/astro-attrs.test.js
@@ -31,6 +31,9 @@ describe('Attributes', async () => {
 			'html-enum-false': { attribute: 'draggable', value: 'false' },
 		};
 
+		// cheerio will unescape the values, so checking that the url rendered unescaped to begin with has to be done manually
+		assert.equal(html.includes("https://example.com/api/og?title=hello&description=somedescription"), true);
+
 		for (const id of Object.keys(attrs)) {
 			const { attribute, value } = attrs[id];
 			const attr = $(`#${id}`).attr(attribute);
diff --git a/packages/astro/test/fixtures/astro-attrs/src/pages/index.astro b/packages/astro/test/fixtures/astro-attrs/src/pages/index.astro
index 82e03a3ae4e4..b7e617e365d2 100644
--- a/packages/astro/test/fixtures/astro-attrs/src/pages/index.astro
+++ b/packages/astro/test/fixtures/astro-attrs/src/pages/index.astro
@@ -5,6 +5,7 @@
 <span id="empty" attr="" />
 <span id="null" attr={null} />
 <span id="undefined" attr={undefined} />
+<span id="url" attr={"https://example.com/api/og?title=hello&description=somedescription"}/>
 <!--
     Per HTML spec, some attributes should be treated as booleans
     These should always render <span async /> or <span /> (without a string value)