Repository navigation
fix(parser): read non-ASCII selectors by byte offset, and don't panic on a microsyntax value that fails to parse - #649
Conversation
… on a microsyntax value that fails to parse
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing. Review the Copilot code review billing settings.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
@codex review |
|
Codex Review: Something went wrong. Try again later by commenting “@codex review”. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
`*ngFor="let i of xs; trackBy: }"` panicked on main; with the swapped
slice it compiled with no error and dropped the `trackBy` binding. A
token that starts no expression is left unread and the value is empty.
Angular's `parsePrimary` reports it ("Unexpected token }") and skips it,
which also gives the value text Angular takes from the AST's span.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… does Angular parses selectors with `_SELECTOR_REGEXP`, whose `\w` has no `u` flag: a name is a run of ASCII `[-\w]`, so `.ação` is the class `a` and then the element `o`, and `<aé [href]>` looks up `a|href` for its sanitizer. An attribute matches only `\[([-.\w*\\$]+)(?:=(["']?)([^\]"']*)\5)?\]`; otherwise the regexp goes on from the char after the `[`. An unquoted value is no longer trimmed. The linker escaping test used `[t='a"b']`, which Angular reads as the element `b`: a quote never reaches a selector value. It now escapes a backslash, which does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Something went wrong. Try again later by commenting “@codex review”. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
Summary
Two places where the template parser slices a string with the wrong offsets: one gives a wrong selector without any error, and both can panic, which aborts the process in a release build (
panic = "abort").Non-ASCII selectors
CssSelector::parsewalks the selector as aVec<char>but slicesselector(a&str) with those char indices as if they were byte offsets. Any non-ASCII character before the end of a name or value shifts every slice after it.When the shifted offset lands inside a multi-byte character it panics instead (
end byte index 2 is not a char boundary), e.g.<ng-content select="é[ü]">orselector: '[é=ü]'. Component and directiveselectorsgo through the same parser.Each char index is now mapped to its byte offset before slicing.
[slot=título]gives["slot", "título"], asCssSelector.parsedoes in@angular/compiler.A microsyntax value that fails to parse
In
parse_directive_keyword_bindings, when the value expression fails to parse,current_end_index()comes before the value's start, and&self.source[expr_start..value_end]panics (byte range starts at 6 but ends at 5):Angular reads the same range with
substring, which swaps a reversed range, so ngtsc reports the parse errors (Unexpected token /,Unexpected character [ñ]) and the compiler keeps going. The slice now takes the two ends in order, and the errors are reported the same way.Test plan
selector.rs:test_parse_non_asciicovers attribute values (quoted and not), a class, an id, an element name and:not()with non-ASCII characters.expression_parser_test.rs:should_report_a_value_that_fails_to_parseparses both values above and expects errors instead of a panic.mainand pass with the change.cargo test --all-features,cargo fmt --all -- --check, andcargo run -p oxc_angular_conformance(1264/1264, snapshot unchanged).