Skip to content

fix(toml): reject control characters in comments - #7348

Open
dvd233 wants to merge 1 commit into
denoland:mainfrom
dvd233:fix/toml-comment-control-characters
Open

dvd233 wants to merge 1 commit into
denoland:mainfrom
dvd233:fix/toml-comment-control-characters

Conversation

@dvd233

@dvd233 dvd233 commented Oct 5, 2026

Copy link
Copy Markdown

Summary

Reject forbidden ASCII control characters while skipping TOML comments. The scanner now throws SyntaxError at the offending character for U+0000–U+0008, U+000B–U+001F, and U+007F. LF and CRLF still terminate comments; a bare CR is rejected.

The six-line production change preserves TAB, ordinary Unicode, EOF comments, hashes and existing escape handling in strings, and skipComments: false. Scanner and public-parser regression tests cover rejection, error location, and these preserved behaviors. Removing the comment-control ignore prefix enables all six existing invalid/control/comment-* fixtures.

This addresses one comment-validation defect tracked by #6663. It is partial progress on that issue and is independent of the multiline-closing-quotes fix in #7347.

Validation

  • Independent 175-case regression suite: 14 passed / 161 failed before the fix; 175 passed / 0 failed afterward. The failing cases exercise the same defect.
  • Comment-only TOML tests plus docs: 698 passed, 0 failed, 105 existing unrelated ignores.
  • Repository formatting and offline browser-compatible entry-point typecheck passed.
  • Exact deno task ok passed in the isolated fork validation run, using the unchanged comment-only source plus a separate validation workflow.

AI assistance was used for implementation, tests, review, and this PR description.

@github-actions github-actions Bot added the toml label Oct 5, 2026
@codecov

codecov Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.04%. Comparing base (f834d02) to head (46ca94d).

Additional details and impacted files
@@           Coverage Diff            @@
##             main    #7348    +/-   ##
========================================
  Coverage   95.04%   95.04%            
========================================
  Files         619      618     -1     
  Lines       52012    51762   -250     
  Branches     9450     9403    -47     
========================================
- Hits        49434    49197   -237     
+ Misses       2031     2022     -9     
+ Partials      547      543     -4     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@dvd233
dvd233 marked this pull request as ready for review October 5, 2026 18:15

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant