Skip to content

[yaml] Fix tag URI decoding, numeric tag RangeErrors, and dead checks - #2630

Merged
kevmoo merged 7 commits into
mainfrom
fix-yaml
Oct 19, 2026
Merged

kevmoo merged 7 commits into
mainfrom
fix-yaml

Conversation

@kevmoo

@kevmoo kevmoo commented Oct 2, 2026 •

Copy link
Copy Markdown
Member

Rationale

  1. Tag URI decoding & shorthand local tag suffixes (Scanner._scanTagUri): Scanner._scanTagUri previously ignored the StringBuffer holding the already-scanned tag head from _scanTagHandle, causing shorthand local tags (such as !custom 123) to be rewritten to the non-specific '!' tag instead of being rejected as undefined tags. Additionally, _scanTagUri passed unvalidated % sequences directly to Uri.decodeFull, leaking uncaught ArgumentError on malformed percent escapes (e.g. !<%>, !a!%, %TAG ! tag:%) and raw FormatException without a SourceSpan on non-UTF-8 percent-encoded sequences (e.g. !<%C3%28>).
  2. Empty and 1-character !!int / !!float scalar RangeErrors (Loader._parseNumberValue): Empty scalars tagged with !!int or !!float (!!int "", - !!int\n, !!float "") threw RangeError on contents.codeUnitAt(0) instead of YamlException, and 1-character !!float scalars (!!float a, !!float ., or valid !!float 0) skipped the if (allowInt && length == 1) branch and threw RangeError on contents.codeUnitAt(1).
  3. Unreachable parser/scanner checks: Parser defined unused _State.BLOCK_NODE_OR_INDENTLESS_SEQUENCE and _State.FLOW_NODE enum states, and Scanner._scanAnchor checked 5 delimiter characters already consumed by _isAnchorChar.

Summary of Changes

  • pkgs/yaml/lib/src/scanner.dart:
    • Append the remaining scanned tag URI substring to buffer in _scanTagUri so shorthand local tag suffixes scanned in _scanTagHandle are preserved, and add _decodeTagUri / _isHexChar to validate %XX escapes and wrap FormatException in YamlException.
    • Simplify _staleSimpleKeys(), remove 5 unreachable delimiter checks in _scanAnchor(), and remove the redundant null check in _isStandardCharacterAt().
  • pkgs/yaml/lib/src/loader.dart:
    • Guard if (length == 0) return null; and handle if (length == 1) for both allowInt and allowFloat in Loader._parseNumberValue.
  • pkgs/yaml/lib/src/parser.dart:
    • Remove unused _State.BLOCK_NODE_OR_INDENTLESS_SEQUENCE and _State.FLOW_NODE enum states and redundant tagToken?.suffix fallback.
  • pkgs/yaml/test/yaml_test.dart, pkgs/yaml/test/yaml_test_suite_test.dart, & pkgs/yaml/CHANGELOG.md:
    • Un-skip YAML 1.2 spec test [Example 10.7] (zero: !!float 0), add regression tests for tag URI decoding and empty/1-char !!int and !!float scalars, update _expectedFailures, and document the fixes under 3.2.0-wip.

Verification

  • dart analyze --fatal-infos: exit 0
  • dart format --output=none --set-exit-if-changed .: exit 0
  • dart test -c source -r compact: exit 0 (595 tests passed)

Append the remaining scanned URI substring to the StringBuffer containing
the initial tag head in Scanner._scanTagUri so shorthand local tags (such
as !custom) are preserved rather than silently rewritten to '!'.

Validate percent-encoding hex digits and wrap FormatException from
Uri.decodeFull in YamlException with a SourceSpan so malformed or
non-UTF-8 percent-encoded tag URIs throw YamlException instead of leaking
raw ArgumentError or FormatException.
@kevmoo
kevmoo requested a review from a team as a code owner October 2, 2026 16:23
@github-actions

github-actions Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

Package publishing

Package Version Status Publish tag (post-merge)
package:yaml ⭐ 3.2.0-wip WIP (no publish necessary)
  • 21 already published.
  • 21 WIP (no publish necessary).

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 updates the YAML scanner to preserve scanned local tag names and throw a YamlException when encountering malformed or non-UTF-8 percent-encoded tag URIs. It includes comprehensive tests for these validation checks and updates the test suite's expected failures. The review feedback points out a minor spelling typo in the new exception message where 'hexidecimal' should be corrected to 'hexadecimal'.

Comment thread pkgs/yaml/lib/src/scanner.dart Resolved
@github-actions

github-actions Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

PR Health

Coverage ✔️
File Coverage
pkgs/yaml/lib/src/loader.dart 💚 98 % ⬆️ 2 %
pkgs/yaml/lib/src/parser.dart 💚 97 % ⬆️ 1 %
pkgs/yaml/lib/src/scanner.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.

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?
yaml Non-Breaking 3.1.4 3.2.0-wip 3.2.0-wip ✔️

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

Unused Dependencies ✔️
Package Status
yaml ✔️ 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.

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
yaml ScalarEvent yaml_node.dart::YamlScalar::internal::scalar
yaml EventType event.dart::Event::type
event.dart::EventType::streamStart
event.dart::EventType::streamEnd
event.dart::EventType::documentStart
event.dart::EventType::documentEnd
event.dart::EventType::alias
event.dart::EventType::scalar
event.dart::EventType::sequenceStart
event.dart::EventType::sequenceEnd
event.dart::EventType::mappingStart
event.dart::EventType::mappingEnd
event.dart::EventType::values
event.dart::Event::new::type
event.dart::ScalarEvent::type

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).

kevmoo added 2 commits October 2, 2026 16:54
…r/scanner checks

- Guard empty strings (`length == 0`) and handle `length == 1` for both
  `allowInt` and `allowFloat` in `Loader._parseNumberValue` so empty or
  invalid single-character `!!int` and `!!float` scalars throw
  `YamlException` instead of `RangeError`, and single-digit `!!float`
  scalars (such as `!!float 0` in YAML 1.2 Example 10.7) parse cleanly.
- Remove unused `_State.BLOCK_NODE_OR_INDENTLESS_SEQUENCE` and
  `_State.FLOW_NODE` states and redundant null-aware `tagToken?.suffix`
  fallback in `Parser`.
- Simplify `Scanner._staleSimpleKeys` (`_indentLevels.last.type`),
  remove 5 unreachable delimiter checks in `Scanner._scanAnchor` for
  characters already consumed by `_isAnchorChar`, and remove the
  redundant null check in `Scanner._isStandardCharacterAt`.
@kevmoo kevmoo changed the title [yaml] Fix tag URI decoding and preserve local tag suffixes [yaml] Fix tag URI decoding, numeric tag RangeErrors, and dead checks Oct 3, 2026
Comment thread pkgs/yaml/test/yaml_test.dart Resolved
@kevmoo
kevmoo merged commit f1a55d9 into main Oct 19, 2026
22 checks passed
@kevmoo
kevmoo deleted the fix-yaml branch October 19, 2026 14:53
copybara-service Bot pushed a commit to dart-lang/sdk that referenced this pull request Oct 20, 2026
…, web

Revisions updated by `dart tools/rev_sdk_deps.dart`.

core (https://github.com/dart-lang/core/compare/4d80610..72e71ca):
  72e71ca0  Mon Oct 19 21:57:11 2026 +0200  Jonas Finnemann Jensen  Optimize `HashSink` and `_MD5Sink.updateHash` (RFC impl) (dart-lang/core#1013)

dartdoc (https://github.com/dart-lang/dartdoc/compare/e1859cf..c0e19cf):
  c0e19cf6  Mon Oct 19 11:41:42 2026 -0700  Konstantin Scheglov  Import analyzer ast with explicit show. (dart-lang/dartdoc#4300)

ecosystem (https://github.com/dart-lang/ecosystem/compare/622ef61..0ab0cfd):
  0ab0cfd  Mon Oct 19 15:44:26 2026 +0200  Moritz  fix(firehose): bump dependency_validator to support analyzer 14 (dart-lang/ecosystem#453)
  44a0768  Wed Oct 14 09:37:29 2026 -0700  Nate Bosch  Improve firehose publish validation log output (dart-lang/ecosystem#468)

http (https://github.com/dart-lang/http/compare/7d7aae2..a8a24a0):
  a8a24a0  Mon Oct 19 13:31:34 2026 -0700  Kevin Moore  docs(web_socket): remove non-user-visible changelog entry (dart-lang/http#2023)
  6f6ac5f  Mon Oct 19 16:29:43 2026 -0400  Mouad Debbar  [http_multi_server] Retry `loopback(0)` when IPv6 port is already bound (dart-lang/http#2025)
  6e91ce3  Tue Oct 13 16:25:39 2026 -0700  Kevin Moore  Sync root README.md package table from pubspec.yaml files (dart-lang/http#2024)

native (https://github.com/dart-lang/native/compare/cc7e4b0..3cbe319):
  3cbe319e3  Tue Oct 13 15:47:18 2026 +0200  Daco Harkes  [hooks_runner] Fail eagerly when file locking is not supported on file system (dart-lang/native#3679)

shelf (https://github.com/dart-lang/shelf/compare/b2cb503..b66665b):
  b66665b  Fri Oct 16 15:12:05 2026 -0700  Kevin Moore  chore: add health workflow and prepare shelf 1.4.3, shelf_router 1.1.5, and shelf_static 1.2.0 (dart-lang/shelf#546)

test (https://github.com/dart-lang/test/compare/9bf69a8..d1afeb1):
  d1afeb14  Fri Oct 16 20:41:51 2026 -0400  Mouad Debbar  Confirmation that `tearDown` is called on timeout (dart-lang/test#2798)
  3f6c2f14  Thu Oct 15 18:21:35 2026 -0700  Jacob MacDonald  Enable asserts in the cli compiler (dart-lang/test#2795)

tools (https://github.com/dart-lang/tools/compare/c1471db..f6c7a2d):
  f6c7a2d62  Mon Oct 19 07:59:24 2026 -0700  Kevin Moore  [pubspec_parse] Preserve input maps and YamlMap source spans (dart-lang/tools#2653)
  f1a55d953  Mon Oct 19 07:53:51 2026 -0700  Kevin Moore  [yaml] Fix tag URI decoding, numeric tag RangeErrors, and dead checks (dart-lang/tools#2630)
  340ded284  Mon Oct 19 07:43:31 2026 -0700  Kevin Moore  [unified_analytics] Prepare for 8.2.0 release (dart-lang/tools#2689)
  a73703543  Fri Oct 16 11:55:03 2026 -0700  Kevin Moore  [unified_analytics] Bump version to 8.2.0-wip (dart-lang/tools#2688)

web (https://github.com/dart-lang/web/compare/63e6662..4e60e61):
  4e60e61  Mon Oct 19 10:02:22 2026 -0700  dependabot[bot]  Bump source-map-js from 1.2.1 to 1.2.2 in /web_generator (dart-lang/web#591)
  7d63637  Mon Oct 19 09:50:26 2026 -0700  Kevin Moore  Add an api.txt golden for package:web (dart-lang/web#590)

[email protected]

Change-Id: Ibe6ac8a8246a27caa8a4846ea1acc78c801ee2ae
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/566781
Commit-Queue: Nate Bosch <[email protected]>
Reviewed-by: Brian Quinlan <[email protected]>
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.

2 participants