Skip to content

Fix crash on malformed nested brace/paren patterns - #68

Open
afonsojanu wants to merge 1 commit into
micromatch:masterfrom
afonsojanu:fix/expand-crash-on-malformed-nested-patterns
Open

Fix crash on malformed nested brace/paren patterns#68
afonsojanu wants to merge 1 commit into
micromatch:masterfrom
afonsojanu:fix/expand-crash-on-malformed-nested-patterns

Conversation

@afonsojanu

Copy link
Copy Markdown

Fixes #60.

braces('{{*(()', { expand: true })
// TypeError: Cannot read properties of undefined (reading 'push')

The crash is in walk(). When it hits a leaf node with a value, it climbs the node's parent chain looking for the nearest ancestor typed brace or root, then pushes onto that ancestor's queue array:

while (block.type !== 'brace' && block.type !== 'root' && block.parent) {
  block = block.parent;
  queue = block.queue;
}
...
queue.push(append(queue.pop(), child.value));

That assumes the ancestor it lands on already has queue set, which normally happens because walk() sets node.queue = [] right when it's called on a node. For this particular pattern, the parser builds a paren node whose parent chain runs through a brace node that walk() never actually visits on its own, since the parser leaves it out of the tree it hands back for input this malformed. So that ancestor's queue stays undefined, and pushing onto it blows up.

I traced this with some throwaway logging rather than guessing: for {{*((), the crash happens exactly when reaching the inner paren node's () value, at which point block.type is 'brace' and block.queue is undefined.

The fix just falls back to an empty array in both places this climb happens, when it lands on a node without a queue. This matches how the rest of this function already treats other malformed patterns (falling back to something reasonable rather than throwing), so {{*(() now expands to ['{{*('] instead of crashing.

Added a test in test/regression.js alongside the existing malformed-pattern cases there, covering the reported pattern plus two related shapes ({{{ and ((()). Confirmed it fails on master and passes with the fix. Ran the full suite locally, 895 passing.

expand() throws "Cannot read properties of undefined (reading 'push')"
on patterns like {{*(() instead of falling back to the literal string
the way other malformed brace patterns already do.

walk() climbs a node's parent chain looking for the nearest ancestor
typed 'brace' or 'root' so it can reuse that ancestor's queue array.
That climb assumes every such ancestor already got its queue
initialized, which normally happens because walk() sets node.queue = []
the moment it's called on a node. For this pattern though, the parser
builds a paren node whose parent chain includes a brace node that
walk() never actually visits directly, since the parser leaves it out
of the tree that gets walked for input this broken. Its queue stays
undefined, and the next push on it blows up.

Falling back to an empty array when the climb lands on a queue-less
node keeps the same "give back the input mostly as-is" behavior this
file already uses elsewhere for other kinds of malformed patterns,
instead of crashing.

Added a regression test in test/regression.js covering the reported
pattern plus two related malformed shapes, and confirmed it fails
without the fix and passes with it. Full suite (895 tests) stays
green.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TypeError: Cannot read properties of undefined (reading 'push')

1 participant