fix(alphatex): prevent TypeError crashes on malformed bend/whammy properties - #2796
Open
otherwiseguy wants to merge 1 commit into
Open
fix(alphatex): prevent TypeError crashes on malformed bend/whammy properties#2796otherwiseguy wants to merge 1 commit into
otherwiseguy wants to merge 1 commit into
Conversation
…perties
The bend (b/be) and whammy (tb/tbe) handler branches scan leading
type/style identifiers with an unbounded loop:
while (typeAndStyle) {
switch (p.arguments!.arguments[tbi].nodeType) {
Two malformed inputs escape this loop's assumptions and raise an
uncaught TypeError out of readScore():
C4 {b invalid (0 4)} TypeError: Cannot read properties of
undefined (reading 'arguments')
C4 {b bend invalid (0 4)} TypeError: Cannot read properties of
undefined (reading 'nodeType')
C4 {tb invalid (0 1)} (same as the first, whammy branch)
Cause: argument parsing only absorbs identifiers that are known values
for the property, so an unknown word such as `invalid` starts a new
property instead. That leaves the bend/whammy property either with no
argument list at all (p.arguments undefined) or with every argument
consumed as a type/style word (index past the end).
Bound the loop by the actual argument count, and return early from
_getBendPoints when the property has no argument list. Malformed input
now flows through the normal diagnostic path (AT212, "Unrecogized
property 'invalid'", with a source position) instead of crashing, and
the parsed-bend paths are untouched.
The three affected tests live in the `errors > at219` block and
previously snapshotted empty diagnostic arrays, because importErrorTest
records whatever readScore() produces — including a crash. Their
snapshots now contain the emitted diagnostic. Note the code is AT212
rather than the block's AT219: the invalid word never reaches the bend
handler as an argument, so it is reported as an unknown property.
All 1115 tests in test/importer, test/model and test/audio pass.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Sorry for the wordy noise, I was definitely not ready to submit this. I was running through test framework seeing if AI could generate a Rust port of the alphaTex parser. I told it I wanted to record any bugs we found for upstream PRs, and it instead immediately did one on my behalf before I could read it and verify the legitimacy of the issues.
Feel free to close if this isn't useful. Sorry again for sending an un-human-verified PR.
Problem
Three malformed alphaTex inputs raise an uncaught
TypeErrorout ofreadScore():Every sibling case in the same test block —
{dy invalid},{ot invalid},{gr (invalid)},{beam invalid}and a dozen more — reports a proper diagnostic instead, so this looks unintended rather than by design.Cause
The bend (
b/be) and whammy (tb/tbe) branches ofAlphaTex1LanguageHandlerscan leading type/style identifiers with an unbounded loop:Argument parsing only absorbs identifiers that are known values for the property, so an unknown word such as
invalidstarts a new property rather than being consumed as an argument. That leaves the bend/whammy property either with no argument list at all (p.argumentsundefined → first crash) or with every argument already consumed as a type/style word (tbipast the end → second crash).Fix
Bound the loop by the actual argument count, and return early from
_getBendPointswhen the property has no argument list (12 lines total). Malformed input now flows through the normal diagnostic path; parsed-bend behaviour is untouched.Before / after for the three inputs: uncaught
TypeError→AT212 "Unrecogized property 'invalid'"with a source position.Tests
The three affected cases live in
errors > at219and previously snapshotted empty diagnostic arrays, becauseimportErrorTestrecords whateverreadScore()produces — including a crash — so the suite stayed green. Their snapshots now contain the emitted diagnostic.Worth noting: the recorded code is
AT212(unknown property), not the block'sAT219(invalid enum value). The invalid word never reaches the bend handler as an argument, so it cannot currently be reported as a bad enum value — ifAT219is the desired diagnostic for these inputs, that would be a follow-up in argument parsing rather than the handler. Happy to move or rename the tests if you'd prefer them outside theat219block.test/importer,test/modelandtest/audiopass (1115 tests).