[markdown] Avoid eager reference link label allocation and enforce 999-char limit by jonasfj · Pull Request #2728 · dart-lang/tools

jonasfj · 2026-10-27T11:10:38Z

Defer slicing and normalizing reference link labels in LinkSyntax.close until a reference link or custom linkBuilder actually needs the label, and enforce CommonMark's 999-character limit on reference link labels.

github-actions · 2026-10-27T11:12:24Z

PR Health

License Headers ✔️

// Copyright (c) 2026, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

Files
no missing headers

All source files should start with a license header.

Unrelated files missing license headers

Files
pkgs/html/test/differential_test.dart
pkgs/html/test/dom_parsing_test.dart
pkgs/html/test/mxss_test.dart
pkgs/html/test/noah_ark_clause_limit_test.dart

This check can be disabled by tagging the PR with skip-license-check.

Breaking changes ✔️

Package	Change	Current Version	New Version	Needed Version	Looking good?
markdown	Non-Breaking	7.3.1	7.4.1-wip	7.4.1-wip	✔️

This check can be disabled by tagging the PR with skip-breaking-check.

Unused Dependencies ✔️

Package	Status
markdown	✔️ All dependencies utilized correctly.

For details on how to fix these, see dependency_validator.

This check can be disabled by tagging the PR with skip-unused-dependencies-check.

Coverage

⚠️

File	Coverage
pkgs/markdown/lib/src/inline_syntaxes/footnote_ref_syntax.dart	💚 100 %
pkgs/markdown/lib/src/inline_syntaxes/link_syntax.dart	💔 98 % ⬇️ 0 %

This check for test coverage is informational (issues shown here will not fail the PR).

This check can be disabled by tagging the PR with skip-coverage-check.

API leaks ✔️

The following packages contain symbols visible in the public API, but not exported by the library. Export these symbols or remove them from your publicly visible API.

Package	Leaked API symbol	Leaking sources

This check can be disabled by tagging the PR with skip-leaking-check.

Changelog Entry ✔️

Package	Changed Files

Changes to files need to be accounted for in their respective changelogs.

This check can be disabled by tagging the PR with skip-changelog-check (for example, for non-user-visible changes when the package version is already at -wip).

gemini-code-assist

Code Review

This pull request optimizes reference link parsing in LinkSyntax by avoiding eager slicing and normalization of link labels, and enforces the 999-character CommonMark limit on reference link labels. The reviewer feedback correctly points out that the 999-character limit applies strictly to the link label, not the link text, and recommends removing the allowLinkBuilder parameter and its associated restrictions on link text length. Additionally, the reviewer suggests improving DartDoc comments to align with Effective Dart guidelines and adding test cases to verify the resolution of links with long text but valid short labels.

gemini-code-assist · 2026-10-27T11:13:16Z

    Map<String, LinkReference> linkReferences, {
    required List<Node> Function() getChildren,
+    bool allowLinkBuilder = true,
  }) {


Remove the unused allowLinkBuilder parameter since the 999-character limit only applies to the link label, and we no longer need to restrict the link builder based on the link text length.

Map<String, LinkReference> linkReferences, { required List<Node> Function() getChildren, }) {

gemini-code-assist · 2026-10-27T11:13:16Z

    String? title, {
    bool? secondary,
+    bool allowLinkBuilder = true,
  }) {


Remove the unused allowLinkBuilder parameter from _tryCreateReferenceLink.

String? title, { bool? secondary, }) {

gemini-code-assist · 2026-10-27T11:13:16Z

      getChildren: getChildren,
+      allowLinkBuilder: allowLinkBuilder,
    );


Remove allowLinkBuilder from the call to _resolveReferenceLink.

Suggested change

getChildren: getChildren,

allowLinkBuilder: allowLinkBuilder,

);

getChildren: getChildren,

);

gemini-code-assist · 2026-10-27T11:13:16Z

+  test('reference link labels over 999 characters are not resolved', () {
+    final label999 = 'a' * 999;
+    final label1000 = 'a' * 1000;
+    expect(
+      markdownToHtml('[$label999]: /url\n\n[text][$label999]'),
+      '<p><a href="/url">text</a></p>\n',
+    );
+    expect(
+      markdownToHtml('[$label999]: /url\n\n[text][$label1000]'),
+      '<p>[text][$label1000]</p>\n',
+    );
+    expect(
+      markdownToHtml('[$label1000]', linkResolver: (name, [_]) => Text(name)),
+      '<p>[$label1000]</p>\n',
+    );
+  });


Add test cases to verify that full reference links with link text over 999 characters (but with a valid label <= 999 characters) are correctly resolved, both with standard link references and with custom link resolvers.

test('reference link labels over 999 characters are not resolved', () { final label999 = 'a' * 999; final label1000 = 'a' * 1000; final text1000 = 'b' * 1000; expect( markdownToHtml('[$label999]: /url\n\n[text][$label999]'), '<a href="/url">text</a>\n', ); expect( markdownToHtml('[$label999]: /url\n\n[text][$label1000]'), '[text][$label1000]\n', ); expect( markdownToHtml('[$label1000]', linkResolver: (name, [_]) => Text(name)), '[$label1000]\n', ); expect( markdownToHtml('[$label999]: /url\n\n[$text1000][$label999]'), '<a href="/url">$text1000</a>\n', ); expect( markdownToHtml('[$text1000][label]', linkResolver: (name, [title]) => Text('$name:$title')), 'label:$text1000\n', ); });

…e keys

davidmorgan

The label allocation is now deferred, and the 999-character cap matches CommonMark. Head's suite passes, and the new regression test fails on the base lib as expected. Output on a set of reference, image, emphasis, footnote and case-folding inputs is unchanged except for labels over 999 characters.

Nested brackets are still quadratic when no link resolves. nodeIndex at inline_parser.dart:157 is computed with _tree.lastIndexWhere before close(), so with no matching reference every ] scans back over the whole tree. That file isn't in the diff, so the comment goes here. nodeIndex is only needed on success: computing it inside getChildren, with a fallback on the success path when getChildren never ran, removes the scan. Timings for '[' * n + 'x' + ']' * n with no references:

n this PR with nodeIndex deferred
8000 326 ms 64 ms
16000 1124 ms 18 ms
32000 5628 ms 25 ms
The timing patch was not run against the suite.

Checked: base eee87b1 and head a79e416; the suite, analyze and format; head's regression tests over the base lib; output diffs on both; and the timings above.

davidmorgan · 2026-10-27T15:42:24Z

+  ) {
+    // A link label can have at most 999 characters inside the square
+    // brackets. See https://spec.commonmark.org/0.30/#link-label.
+    if (labelEnd <= labelStart || labelEnd - labelStart > 999) {


Behaviour change: x [] y with a linkResolver used to call the resolver with an empty name, and render its result. Now the resolver isn't called and [] stays literal. That matches CommonMark, since a label needs a non-whitespace character, but it may be worth a CHANGELOG line next to the 999 entry.

davidmorgan · 2026-10-27T15:42:24Z

-      // TODO(srawlins): only check 999 characters, for performance reasons?
+      // A link label can have at most 999 characters inside the square
+      // brackets. See https://spec.commonmark.org/0.30/#link-label.
+      if (parser.pos - start > 999) return null;


The inline cap counts raw characters. The definition side, LinkParser.parseLabel, still miscounts escapes, so [<998 a>\[]: /url is still accepted as a definition, as before. A shortcut reference to it, [<998 a>\[], used to link; now it's over the inline cap and renders as literal text. It's an edge case, but the two sides now disagree at the boundary. Counting the same way in parseLabel would line them up.

github-actions Bot added the package:markdown label Oct 27, 2026

gemini-code-assist Bot reviewed Oct 27, 2026

View reviewed changes

jonasfj force-pushed the markdown_reference_link_label branch from 4d9d2d4 to 0628223 Compare October 27, 2026 12:47

jonasfj added 3 commits October 27, 2026 13:01

[markdown] Enforce 999-character limit on reference link labels

da711e0

[markdown] Avoid normalizing or sanitizing unused reference link labels

0f00803

[markdown] Check footnote label and empty map before scanning footnot…

a79e416

…e keys

jonasfj force-pushed the markdown_reference_link_label branch from 0628223 to a79e416 Compare October 27, 2026 13:42

jonasfj marked this pull request as ready for review October 27, 2026 13:59

jonasfj requested a review from srawlins as a code owner October 27, 2026 13:59

davidmorgan self-requested a review October 27, 2026 14:31

davidmorgan reviewed Oct 27, 2026

View reviewed changes

jonasfj mentioned this pull request Oct 28, 2026

[markdown] WIP: Fix quadratic, exponential, and deep-recursion #2737

Draft

jonasfj closed this Oct 30, 2026

Navigation Menu

Repository navigation

[markdown] Avoid eager reference link label allocation and enforce 999-char limit - #2728

[markdown] Avoid eager reference link label allocation and enforce 999-char limit#2728
jonasfj wants to merge 3 commits into
mainfrom
markdown_reference_link_label

jonasfj commented Oct 27, 2026

Uh oh!

github-actions Bot commented Oct 27, 2026 •

edited

Loading

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

Uh oh!

gemini-code-assist Bot Oct 27, 2026

Uh oh!

Uh oh!

Uh oh!

gemini-code-assist Bot Oct 27, 2026

Uh oh!

gemini-code-assist Bot Oct 27, 2026

Uh oh!

gemini-code-assist Bot Oct 27, 2026

Uh oh!

davidmorgan left a comment

Uh oh!

davidmorgan Oct 27, 2026

Uh oh!

davidmorgan Oct 27, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

jonasfj commented Oct 27, 2026

Uh oh!

github-actions Bot commented Oct 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Health

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

gemini-code-assist Bot Oct 27, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

gemini-code-assist Bot Oct 27, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Oct 27, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Oct 27, 2026

Choose a reason for hiding this comment

Uh oh!

davidmorgan left a comment

Choose a reason for hiding this comment

Uh oh!

davidmorgan Oct 27, 2026

Choose a reason for hiding this comment

Uh oh!

davidmorgan Oct 27, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions Bot commented Oct 27, 2026 •

edited

Loading