Skip to content

markdown: escape alt text in images - #2478

Merged
srawlins merged 3 commits into
mainfrom
image
Jul 29, 2026
Merged

markdown: escape alt text in images#2478
srawlins merged 3 commits into
mainfrom
image

Conversation

@srawlins

Copy link
Copy Markdown
Member

See b/536194819

@github-actions

github-actions Bot commented Jul 23, 2026

Copy link
Copy Markdown

Package publishing

If you have publishing permissions, you can use the links below to publish the changes after merging this PR.

Package Version Status Publish tag (post-merge)
package:api_summary 0.1.0-wip WIP (no publish necessary)
package:bazel_worker 1.1.5 already published at pub.dev
package:benchmark_harness 2.4.0 already published at pub.dev
package:boolean_selector 2.1.2 already published at pub.dev
package:browser_launcher 1.2.0-wip WIP (no publish necessary)
package:cli_config 0.2.1-wip WIP (no publish necessary)
package:cli_util 0.5.2 already published at pub.dev
package:clock 1.1.3-wip WIP (no publish necessary)
package:code_builder 4.12.0-wip WIP (no publish necessary)
package:coverage 1.15.1 already published at pub.dev
package:csslib 1.0.2 already published at pub.dev
package:extension_discovery 2.1.0 already published at pub.dev
package:file 7.0.2-wip WIP (no publish necessary)
package:file_testing 3.1.0-wip WIP (no publish necessary)
package:glob 2.1.4-wip WIP (no publish necessary)
package:graphs 2.4.0-wip WIP (no publish necessary)
package:html 0.15.7-wip WIP (no publish necessary)
package:io 1.1.0-wip WIP (no publish necessary)
package:json_rpc_2 4.1.0 already published at pub.dev
package:markdown 7.4.0 ready to publish markdown-v7.4.0
package:mime 2.1.0-wip WIP (no publish necessary)
package:oauth2 2.0.5 already published at pub.dev
package:package_config 3.0.0 already published at pub.dev
package:pool 1.5.3-wip WIP (no publish necessary)
package:process 5.0.5 (error) pubspec version (5.0.5) and changelog (5.0.6-wip) don't agree
package:pub_semver 2.2.0 already published at pub.dev
package:pubspec_parse 1.6.0-wip WIP (no publish necessary)
package:source_map_stack_trace 2.1.3-wip WIP (no publish necessary)
package:source_maps 0.10.14-wip WIP (no publish necessary)
package:source_span 1.10.2 already published at pub.dev
package:sse 4.2.1-wip WIP (no publish necessary)
package:stack_trace 1.12.2-wip (error) pubspec version (1.12.2-wip) and changelog (1.12.2-dev) don't agree
package:stream_channel 2.1.4 already published at pub.dev
package:stream_transform 2.1.2-wip WIP (no publish necessary)
package:string_scanner 1.4.2-wip WIP (no publish necessary)
package:term_glyph 1.2.3-wip WIP (no publish necessary)
package:test_reflective_loader 0.6.0 ready to publish test_reflective_loader-v0.6.0
package:timing 1.0.2 already published at pub.dev
package:unified_analytics 8.0.16 already published at pub.dev
package:watcher 1.2.2-wip WIP (no publish necessary)
package:yaml 3.1.4-wip WIP (no publish necessary)
package:yaml_edit 2.2.4 already published at pub.dev

Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automation.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request ensures that image description text is properly escaped when assigned to the HTML 'alt' attribute, addressing potential XSS vulnerabilities. It introduces a new utility function, 'escapeAttributeCharactersValue', to escape quotes and angle brackets without double-escaping existing ampersands, and adds several test cases to verify this behavior. The review feedback recommends optimizing the utility function by utilizing Dart's 'HtmlEscape' with a custom 'HtmlEscapeMode' instead of chained 'replaceAll' calls, and warns of a potential issue where a null 'alt' attribute could be converted to the literal string 'null' during mapping.

Comment thread pkgs/markdown/lib/src/util.dart
Comment thread pkgs/markdown/lib/src/inline_syntaxes/image_syntax.dart Outdated
@github-actions

github-actions Bot commented Jul 23, 2026

Copy link
Copy Markdown

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.

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

Unused Dependencies ⚠️
Package Status
markdown
❗ Show Issues
These packages may be unused, or you may be using assets from these packages:
* meta

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/image_syntax.dart 💚 100 %
pkgs/markdown/lib/src/util.dart 💚 100 %

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.

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.

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.

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

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

@srawlins
srawlins requested a review from natebosch July 27, 2026 21:43

@natebosch natebosch left a comment

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 me on the buganizer?

Comment thread pkgs/markdown/lib/src/inline_syntaxes/image_syntax.dart Outdated
Comment on lines +29 to +30
/// Unlike [escapeHtmlAttribute], this does not escape `&` (ampersand) to
/// avoid double-escaping already-escaped HTML entities.

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.

Why is it that we end up with somethign here which is partially escaped? Where would we have escaped & but not these others?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah this is awkward. By the time the alt text has been put into an element's textContent, it has already passed through EscapeHtmlSyntax, emitting escaped Text nodes. So if you have input like ![a & " b](source), then the text that comes out of node.textContent, in the edited lines above, has a & " b.

This escaping is done early because we parse the alt text into more inline nodes, which is allowed by the spec. The spec allows, for example ![foo *bar*](source), such that the description has inline formatting. In HTML the inline formatting is dropped, producing foo bar alt text. But the model is supposed to parse and keep track of that inner parsing (it also might be required for... correctly parsing erroneous text?).

@srawlins
srawlins merged commit 0fc3b0e into main Jul 29, 2026
24 checks passed
@srawlins
srawlins deleted the image branch July 29, 2026 17:02
copybara-service Bot pushed a commit to dart-lang/sdk that referenced this pull request Jul 29, 2026
* markdown: escape alt text in images
  (dart-lang/tools#2478)

Change-Id: I1d13ab0b08a5b1a957b6fef082b2fc3cfaf0c1c8
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/528940
Reviewed-by: Nate Bosch <nbosch@google.com>
Auto-Submit: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Nate Bosch <nbosch@google.com>
copybara-service Bot pushed a commit to dart-lang/sdk that referenced this pull request Jul 30, 2026
Revisions updated by `dart tools/rev_sdk_deps.dart`.

Rolls a subset of the packages which were rolled and reverted in
https://dart-review.googlesource.com/c/sdk/+/529340

`test` is held back due to triggering a VM issue with some flutter
engine tests.

`dartdoc` is being rolled in
https://dart-review.googlesource.com/c/sdk/+/529460

As was the case with the reverted CL, `http` is being rolled in
https://dart-review.googlesource.com/c/sdk/+/524264

core (https://github.com/dart-lang/core/compare/25926e8..fe516ee):
  fe516ee1  Mon Jul 27 20:19:16 2026 +0200  Moritz  Consolidate no-response workflow into no_response.yml (dart-lang/core#982)
  2deeeade  Mon Jul 27 18:05:47 2026 +0530  Abhishak Kumar Malviya  fix: async cache storing exception fixed (dart-lang/core#548)
  bed8f40e  Fri Jul 24 11:29:13 2026 -0700  Nate Bosch  Add `value` getter to `Result` (dart-lang/core#969)

ecosystem (https://github.com/dart-lang/ecosystem/compare/848b3bf..edfdb3b):
  edfdb3b  Thu Jul 23 17:27:43 2026 +0200  Moritz  Add want-lgtm reusable workflow (dart-lang/ecosystem#432)
  a6c8ef0  Thu Jul 23 14:29:53 2026 +0200  Moritz  Validate target PR number and comment ownership in post_summaries workflow (dart-lang/ecosystem#434)
  80a9928  Thu Jul 23 13:49:48 2026 +0200  Moritz  Fix canary workflow (dart-lang/ecosystem#433)
  910d668  Wed Jul 15 18:04:05 2026 +0200  Moritz  Pass GITHUB_TOKEN to firehose:comment steps in workflows (dart-lang/ecosystem#431)
  980c6df  Sat Jul 11 00:20:55 2026 +0200  Moritz  Fix `-WIP` ending detection (dart-lang/ecosystem#430)
  60c0f67  Wed Jul 8 20:38:35 2026 +0200  Moritz  Run `pub get` before `dart format` in groundskeeper (dart-lang/ecosystem#429)
  bcef699  Tue Jul 7 10:17:24 2026 +0200  Moritz  Fix `groundskeeper` workflow (dart-lang/ecosystem#428)
  77def11  Fri Jul 3 11:43:12 2026 +0200  Moritz  Simplify groundskeeper (dart-lang/ecosystem#426)
  3dd606a  Wed Jul 1 18:19:25 2026 +0000  dependabot[bot]  Bump the github-actions group with 3 updates (dart-lang/ecosystem#427)
  d6ec975  Wed Jul 1 14:41:23 2026 +0200  Moritz  Add changelog updater (dart-lang/ecosystem#424)
  f8a4fbb  Wed Jul 1 10:00:59 2026 +0200  Moritz  Add `Groundskeeper` workflow (dart-lang/ecosystem#425)

i18n (https://github.com/dart-lang/i18n/compare/d0683bd..e1b5a79):
  e1b5a798  Tue Jul 14 13:10:29 2026 +0200  Moritz  Add weekday API (dart-lang/i18n#1071)
  04b78e38  Thu Jul 9 14:16:22 2026 +0200  Moritz  Add stale workflow (dart-lang/i18n#1069)
  01e20101  Thu Jul 9 13:42:08 2026 +0200  Moritz  Add stale workflow
  03efda51  Thu Jul 9 13:19:31 2026 +0200  Moritz  Update ICU4X (dart-lang/i18n#1068)
  4d8a1b39  Tue Jul 7 11:23:06 2026 +0200  Moritz  feat(intl4x): add locale.dart entrypoint exposing only Locale (dart-lang/i18n#1067)
  b2b7ce08  Mon Jul 6 03:23:28 2026 -0700  Copybara-Service  Merge pull request `#1038` from donny-dont:patch-1
  cca1dc44  Mon Jul 6 03:10:59 2026 -0700  Copybara-Service  Merge pull request `#1040` from suxoikorm:main
  28352851  Mon Jul 6 02:40:25 2026 -0700  Copybara-Service  Merge pull request `#1062` from AbdeMohlbi:issue_116
  9f8e7998  Mon Jul 6 02:16:05 2026 -0700  Copybara-Service  Merge pull request `#1050` from LaijieJi:patch-1
  be8e2c0d  Fri Jul 3 14:50:28 2026 +0200  Moritz  Merge branch 'main' into issue_116
  8b119830  Fri Jul 3 14:33:33 2026 +0200  Moritz  Merge branch 'main' into patch-1
  96aab97a  Fri Jul 3 14:30:55 2026 +0200  Moritz  Merge branch 'main' into main
  9e700b2a  Thu Jul 2 09:37:45 2026 +0200  Moritz  Merge branch 'main' into patch-1
  a6f174b8  Wed Jul 1 08:32:07 2026 -0700  Googler  No public description
  fb1feb18  Wed Jul 1 08:32:33 2026 -0700  Copybara-Service  Merge pull request `#1058` from dart-lang:upgrade_analyzer
  77926a26  Thu May 7 20:52:55 2026 +0100  abdessalem  gemini has a point
  c2f96cba  Thu May 7 20:45:40 2026 +0100  abdessalem  revert the formating changes
  ebb0a441  Thu May 7 20:41:40 2026 +0100  abdessalem  update select documentation to clarify other fallback behavior and when `ArgumentError` is thrown
  b13c2d48  Mon Mar 9 10:35:50 2026 +0100  Moritz  Merge branch 'main' into patch-1
  6581675f  Mon Feb 9 09:13:25 2026 +0100  Laijie  Fix typo in number_format.dart documentation
  69d1e6a6  Thu Jan 29 15:25:06 2026 +0400  Luka Katsadze  Merge branch 'main' into main
  55a61fa6  Fri Dec 19 15:51:58 2025 -0800  Don Olmstead  Escape text direction code points
  a140f2ef  Mon Dec 29 21:42:09 2025 +0400  Luka Katsadze  fix: correct formatting of Georgian Lari symbol entry in changelog
  76a370c5  Mon Dec 29 21:34:35 2025 +0400  Luka Katsadze  chore: update changelog
  9df0c191  Mon Dec 29 21:31:39 2025 +0400  Luka Katsadze  feat: add georgian lari symbol support

shelf (https://github.com/dart-lang/shelf/compare/71248e7..6918a76):
  6918a76  Mon Jul 20 18:45:56 2026 -0700  Kevin Moore  feat(compliance): add GitHub Actions Job Summary to test runs (dart-lang/shelf#527)
  e9c742d  Mon Jul 20 20:28:03 2026 +0300  Yusuf İhsan Görgel  fix: join multiple Cookie header values with '; ' instead of ',' (`#521`) (dart-lang/shelf#536)
  833433e  Mon Jul 6 16:59:42 2026 -0700  Kevin Moore  chore: standardize min SDK to ^3.9.0 and unify lints
  7af986e  Wed Jul 1 12:27:06 2026 +0000  dependabot[bot]  Bump the github-actions group with 2 updates (dart-lang/shelf#535)

tools (https://github.com/dart-lang/tools/compare/3f850c4..b827a6e):
  b827a6e3  Wed Jul 29 10:18:52 2026 -0700  Nate Bosch  Use -- to separate chrome URLs (dart-lang/tools#2475)
  0fc3b0e7  Wed Jul 29 10:02:32 2026 -0700  Sam Rawlins  markdown: escape alt text in images (dart-lang/tools#2478)
  01f45d85  Thu Jul 23 16:49:10 2026 -0700  Nate Bosch  [glob] Limit expansion during option flattening (dart-lang/tools#2477)
  acc91d65  Thu Jul 23 16:00:59 2026 -0700  Kevin Moore  Add tests for PoolResource and close, and fix allowRelease leak (dart-lang/tools#2368)
  0e54bd66  Thu Jul 23 15:25:22 2026 -0700  Nate Bosch  Parse hosted URLs as URLs (dart-lang/tools#2476)
  7e124963  Wed Jul 22 15:01:34 2026 -0700  Sam Rawlins  Markdown: optimize email regex (dart-lang/tools#2474)
  906d9d4e  Wed Jul 22 13:25:24 2026 -0700  Sam Rawlins  Markdown: More tag filter tests (dart-lang/tools#2473)
  588fbad4  Wed Jul 22 10:54:42 2026 -0700  Sam Rawlins  Expand HTML filter regexp to include more characters that would denote an HTML tag (dart-lang/tools#2472)
  e4f20cc7  Wed Jul 22 09:33:59 2026 -0700  Sam Rawlins  Respect enableTagfilter when inlineOnly is passed (dart-lang/tools#2471)
  c4082344  Wed Jul 22 07:41:06 2026 -0700  Jacob MacDonald  add legend to multiselect dialogs (dart-lang/tools#2470)
  a2ecf08f  Thu Jul 16 09:30:49 2026 -0700  Jacob MacDonald  add an event type for events coming from package:skills (dart-lang/tools#2468)
  6a85977a  Mon Jul 13 10:12:17 2026 -0700  Kevin Moore  [api_summary] Support latest analyzer version (dart-lang/tools#2460)
  58b0fd0a  Mon Jul 13 11:01:18 2026 +0200  Moritz  Add no-response workflow (dart-lang/tools#2459)
  dc9ef3a9  Wed Jul 8 17:03:20 2026 -0700  Kevin Moore  chore(html): use MIT license header instead of BSD (dart-lang/tools#2458)

web (https://github.com/dart-lang/web/compare/eb8c3fc..12a9ca2):
  12a9ca2  Sun Jul 19 19:27:25 2026 -0700  Kevin Moore  test(js_interop_gen): reformat integration test goldens for dart_style 3.1.12 (dart-lang/web#565)
  6b5e93a  Wed Jul 1 06:55:34 2026 +0000  dependabot[bot]  Bump actions/checkout from 6.0.2 to 7.0.0 in the github-actions group (dart-lang/web#563)
  b8f47ab  Tue Jun 30 16:50:11 2026 -0700  Kevin Moore  [chore] Improve notice about updating the js_type_supertypes.dart (dart-lang/web#562)

R=alexmarkov@google.com

Change-Id: I410cb87fd72a63e4deacaad61206fbbdf6937f5f
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/529520
Commit-Queue: Nate Bosch <nbosch@google.com>
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants