Skip to content

Commit

Permalink
prevent URLs in attributes being escaped (#9820)
Browse files Browse the repository at this point in the history
* 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>
  • Loading branch information
4 people authored Mar 13, 2024
1 parent 2db25c0 commit 8edc42a
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 0 deletions.
5 changes: 5 additions & 0 deletions .changeset/quick-islands-ring.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"astro": patch
---

Prevents fully formed URLs in attributes from being escaped
14 changes: 14 additions & 0 deletions packages/astro/src/runtime/server/render/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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}`);
Expand Down Expand Up @@ -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;
}
}
3 changes: 3 additions & 0 deletions packages/astro/test/astro-attrs.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 8edc42a

Please sign in to comment.