Skip to content

fix: escape character references in autolink destinations #4053

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
UziTech merged 6 commits into from
Sep 4, 2026
Merged

fix: escape character references in autolink destinations #4053

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
fc08928
fix: escape character references in autolink destinations
Kjubikstronk Aug 15, 2026
509451c
fix: escape an ampersand that cannot start a reference
Kjubikstronk Aug 17, 2026
cbb6c53
test: cover link destination escaping with spec fixtures
Kjubikstronk Aug 18, 2026
5a1b3aa
test: drop unit tests now covered by the spec fixtures
Kjubikstronk Aug 18, 2026
f148d5b
test: use the suggested link destination cases
Kjubikstronk Aug 18, 2026
47a0ca3
test: note which cases do not match CommonMark
Kjubikstronk Aug 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 10 additions & 6 deletions src/Renderer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,18 +157,22 @@ export class _Renderer<ParserOutput = string, RendererOutput = string> {
return `<del>${this.parser.parseInline(tokens)}</del>` as RendererOutput;
}

link({ href, title, tokens }: Tokens.Link): RendererOutput {
const text = this.parser.parseInline(tokens) as string;
link({ href, title, text, tokens, autolink }: Tokens.Link): RendererOutput {
// References are not resolved inside an autolink, so every `&` there is
// literal. Elsewhere only an `&` that cannot start one needs escaping.
const parsedText = autolink
? escapeHtmlEntities(text, true)
: this.parser.parseInline(tokens) as string;
const cleanHref = cleanUrl(href);
if (cleanHref === null) {
return text as RendererOutput;
return parsedText as RendererOutput;
}
href = cleanHref;
href = escapeHtmlEntities(cleanHref, autolink);
let out = '<a href="' + href + '"';
if (title) {
out += ' title="' + (escapeHtmlEntities(title)) + '"';
}
out += '>' + text + '</a>';
out += '>' + parsedText + '</a>';
return out as RendererOutput;
}

Expand All @@ -182,7 +186,7 @@ export class _Renderer<ParserOutput = string, RendererOutput = string> {
}
href = cleanHref;

let out = `<img src="${href}" alt="${escapeHtmlEntities(text)}"`;
let out = `<img src="${escapeHtmlEntities(href)}" alt="${escapeHtmlEntities(text)}"`;
if (title) {
out += ` title="${escapeHtmlEntities(title)}"`;
}
Expand Down
2 changes: 2 additions & 0 deletions src/Tokenizer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -914,6 +914,7 @@ export class _Tokenizer<ParserOutput = string, RendererOutput = string> {
raw: cap[0],
text,
href,
autolink: true,
tokens: [
{
type: 'text',
Expand Down Expand Up @@ -951,6 +952,7 @@ export class _Tokenizer<ParserOutput = string, RendererOutput = string> {
raw: cap[0],
text,
href,
autolink: true,
tokens: [
{
type: 'text',
Expand Down
5 changes: 5 additions & 0 deletions src/Tokens.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,11 @@ export namespace Tokens {
title?: string | null;
text: string;
tokens: Token[];
/**
* Set for autolinks and extended (GFM) urls, where character references are
* not resolved, so the destination and text are literal.
*/
autolink?: boolean;
}

export interface List {
Expand Down
7 changes: 7 additions & 0 deletions test/specs/new/link_destination_character_references.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<p><a href="https://example.com/?x=1&amp;lt;2">https://example.com/?x=1&amp;lt;2</a></p>
<p><a href="https://example.com/?x=1&amp;lt;2">https://example.com/?x=1&amp;lt;2</a></p>
<p><a href="https://example.com/?a=1&amp;b=2">t</a></p>
<p><a href="http://example.com?foo=1&amp;bar=2">example</a></p>
<p><img src="http://example.com?foo=1&amp;bar=2" alt="i"></p>
<p><a href="http://example.com?a=1&lt;2">t</a></p>

@UziTech UziTech Aug 18, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to this CommonMark demo this line should be

<p><a href="http://example.com?a=1%3C2">t</a></p>

@UziTech UziTech Aug 18, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you use these as the tests:

---
renderExact: true
---
https://example.com/?x=1&lt;2

https://example.com/?y=1&amp;2

https://example.com/?a=1&b=2

<https://example.com/?x=1&lt;2>

<https://example.com/?y=1&amp;2>

<https://example.com/?a=1&b=2>

[https://example.com/?x=1&lt;2](https://example.com/?x=1&lt;2)

[https://example.com/?y=1&amp;2](https://example.com/?y=1&amp;2)

[https://example.com/?a=1&b=2](https://example.com/?a=1&b=2)

[https://example.com/?x=1&lt;2][link1]

[https://example.com/?y=1&amp;2][link2]

[https://example.com/?a=1&b=2][link3]

![https://example.com/?x=1&lt;2](https://example.com/?x=1&lt;2)

![https://example.com/?y=1&amp;2](https://example.com/?y=1&amp;2)

![https://example.com/?a=1&b=2](https://example.com/?a=1&b=2)

[link1]: https://example.com/?x=1&lt;2
[link2]: https://example.com/?y=1&amp;2
[link3]: https://example.com/?a=1&b=2

<p><a href="https://example.com/?x=1&amp;lt;2">https://example.com/?x=1&amp;lt;2</a></p>
<p><a href="https://example.com/?y=1&amp;amp;2">https://example.com/?y=1&amp;amp;2</a></p>
<p><a href="https://example.com/?a=1&amp;b=2">https://example.com/?a=1&amp;b=2</a></p>
<p><a href="https://example.com/?x=1&amp;lt;2">https://example.com/?x=1&amp;lt;2</a></p>
<p><a href="https://example.com/?y=1&amp;amp;2">https://example.com/?y=1&amp;amp;2</a></p>
<p><a href="https://example.com/?a=1&amp;b=2">https://example.com/?a=1&amp;b=2</a></p>
<p><a href="https://example.com/?x=1%3C2">https://example.com/?x=1&lt;2</a></p>
<p><a href="https://example.com/?y=1&amp;2">https://example.com/?y=1&amp;2</a></p>
<p><a href="https://example.com/?a=1&amp;b=2">https://example.com/?a=1&amp;b=2</a></p>
<p><a href="https://example.com/?x=1%3C2">https://example.com/?x=1&lt;2</a></p>
<p><a href="https://example.com/?y=1&amp;2">https://example.com/?y=1&amp;2</a></p>
<p><a href="https://example.com/?a=1&amp;b=2">https://example.com/?a=1&amp;b=2</a></p>
<p><img src="https://example.com/?x=1%3C2" alt="https://example.com/?x=1&lt;2"></p>
<p><img src="https://example.com/?y=1&amp;2" alt="https://example.com/?y=1&amp;2"></p>
<p><img src="https://example.com/?a=1&amp;b=2" alt="https://example.com/?a=1&amp;b=2"></p>

@UziTech UziTech Aug 18, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated the tests to add reflinks and show the difference between the href and text of some

@Kjubikstronk Kjubikstronk Aug 18, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Swapped in your cases, thanks — the reflinks and the href/text split make the autolink-vs-inline difference much clearer than what I had.

On the %3C one: you're right that CommonMark gives http://example.com?a=1%3C2 there, but that line comes out the same on master as on this branch. I diffed the whole fixture both ways — this PR only changes the six autolink lines and the ?a=1&b=2 cases, where master emits a raw & in the href. The three &lt; inline/reference/image destinations are byte-identical before and after.

So the missing percent-encoding looks like a separate gap rather than something I've introduced. Happy to take it on, either here or as its own PR — just say which you'd prefer, since it's a different code path from the entity escaping.

@UziTech UziTech Aug 19, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a comment in the file about the lines that don't match CommonMark

@Kjubikstronk Kjubikstronk Aug 19, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added it to the frontmatter — front-matter runs the block through YAML, so # comments are stripped and never reach the rendered output. The fixture still passes byte for byte.

It calls out that the &lt; inline links, reference links and images are the ones that differ: CommonMark resolves the reference and percent-encodes it to ?x=1%3C2 where marked keeps ?x=1&lt;2. I noted that this comes from URL encoding rather than from the escaping in this PR, and that it is unchanged on master, so nobody reads those lines later as something this PR introduced.

<p><a href="https://foo.bar.baz/test?q=hello&amp;id=22&amp;boolean">https://foo.bar.baz/test?q=hello&amp;id=22&amp;boolean</a></p>
16 changes: 16 additions & 0 deletions test/specs/new/link_destination_character_references.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
---
renderExact: true
---
<https://example.com/?x=1&lt;2>

https://example.com/?x=1&lt;2

[t](https://example.com/?a=1&amp;b=2)

[example](http://example.com?foo=1&bar=2)

![i](http://example.com?foo=1&bar=2)

[t](http://example.com?a=1&lt;2)

<https://foo.bar.baz/test?q=hello&id=22&boolean>
4 changes: 4 additions & 0 deletions test/unit/Lexer.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2042,6 +2042,7 @@ paragraph
raw: '<https://example.com>',
text: 'https://example.com',
href: 'https://example.com',
autolink: true,
tokens: [
{
type: 'text',
Expand All @@ -2064,6 +2065,7 @@ paragraph
raw: '<test@example.com>',
text: 'test@example.com',
href: 'mailto:test@example.com',
autolink: true,
tokens: [
{
type: 'text',
Expand All @@ -2085,6 +2087,7 @@ paragraph
raw: 'https://example.com',
text: 'https://example.com',
href: 'https://example.com',
autolink: true,
tokens: [
{
type: 'text',
Expand All @@ -2107,6 +2110,7 @@ paragraph
raw: 'test@example.com',
text: 'test@example.com',
href: 'mailto:test@example.com',
autolink: true,
tokens: [
{
type: 'text',
Expand Down
58 changes: 58 additions & 0 deletions test/unit/marked.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,64 @@ describe('marked unit', () => {
setOptions(getDefaults());
});

describe('Character references in link destinations', () => {
// Character references are not resolved inside an autolink, so a `&` there
// is literal and has to be escaped. These assert on the exact output
// because the spec runner compares parsed HTML, where `&lt;` and `&amp;lt;`
// in an attribute are indistinguishable.
// https://github.com/markedjs/marked/issues/4052

it('should escape an ampersand in an autolink', () => {
Comment thread
UziTech marked this conversation as resolved.
Resolved
assert.strictEqual(
marked.parse('<https://example.com/?x=1&lt;2>').trim(),
'<p><a href="https://example.com/?x=1&amp;lt;2">https://example.com/?x=1&amp;lt;2</a></p>',
);
});

it('should escape an ampersand in an extended url', () => {
assert.strictEqual(
marked.parse('https://example.com/?x=1&lt;2').trim(),
'<p><a href="https://example.com/?x=1&amp;lt;2">https://example.com/?x=1&amp;lt;2</a></p>',
);
});

it('should keep the destination of an inline link unchanged', () => {
assert.strictEqual(
marked.parse('[t](https://example.com/?a=1&amp;b=2)').trim(),
'<p><a href="https://example.com/?a=1&amp;b=2">t</a></p>',
);
});

it('should escape an ampersand that cannot start a reference', () => {
assert.strictEqual(
marked.parse('[example](http://example.com?foo=1&bar=2)').trim(),
'<p><a href="http://example.com?foo=1&amp;bar=2">example</a></p>',
);
});

it('should escape an ampersand in an image source', () => {
assert.strictEqual(
marked.parse('![i](http://example.com?foo=1&bar=2)').trim(),
'<p><img src="http://example.com?foo=1&amp;bar=2" alt="i"></p>',
);
});

it('should leave a reference in an inline destination alone', () => {
assert.strictEqual(
marked.parse('[t](http://example.com?a=1&lt;2)').trim(),
'<p><a href="http://example.com?a=1&lt;2">t</a></p>',
);
});

it('should match the CommonMark autolink example', () => {
// https://spec.commonmark.org/0.31.2/#example-595
assert.strictEqual(
marked.parse('<https://foo.bar.baz/test?q=hello&id=22&boolean>').trim(),
'<p><a href="https://foo.bar.baz/test?q=hello&amp;id=22&amp;boolean">https://foo.bar.baz/test?q=hello&amp;id=22&amp;boolean</a></p>',
);
});
});

describe('Test paragraph token type', () => {
it('should use the "paragraph" type on top level', () => {
const md = 'A Paragraph.\n\n> A blockquote\n\n- list item\n';
Expand Down