Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incomplete switch statement causes espree to hang/crash #146

Closed
xjamundx opened this issue May 29, 2015 · 6 comments
Closed

Incomplete switch statement causes espree to hang/crash #146

xjamundx opened this issue May 29, 2015 · 6 comments
Labels

Comments

@xjamundx
Copy link
Contributor

The following code will crash espree (so long as it's followed by a newline):

switch(x) {  case 'case':  break;

It's very easy to reproduce:

> require('espree').parse("switch(x) {  case 'case':  break;\n")
FATAL ERROR: CALL_AND_RETRY_LAST Allocation failed - process out of memory
[1]    63533 abort      node
@nzakas
Copy link
Member

nzakas commented May 29, 2015

Yuck. I'm stuck away from a computer, any interest in investigating?

@xjamundx
Copy link
Contributor Author

Yes I'll try to find out what's up sometime later today.

@xjamundx
Copy link
Contributor Author

  1. Verified it happens in esprima as well
  2. When I run node --trace I get miles of
16:                *parseSourceElement+24(this=0x1a1581304121 <undefined>) {
  16:                } -> 0x1a1581304121 <undefined>
  16:                *parseSourceElement+24(this=0x1a1581304121 <undefined>) {
  16:                } -> 0x1a1581304121 <undefined>
  16:                *parseSourceElement+24(this=0x1a1581304121 <undefined>) {
  16:                } -> 0x1a1581304121 <undefined>
  16:                *parseSourceElement+24(this=0x1a1581304121 <undefined>) {
  16:                } -> 0x1a1581304121 <undefined>
  16:                *parseSourceElement+24(this=0x1a1581304121 <undefined>) {
  16:                } -> 0x1a1581304121 <undefined>
  16:                *parseSourceElement+24(this=0x1a1581304121 <undefined>) {
  16:                } -> 0x1a1581304121 <undefined>
  16:                *parseSourceElement+24(this=0x1a1581304121 <undefined>) {
  16:                } -> 0x1a1581304121 <undefined>
  16:                *parseSourceElement+24(this=0x1a1581304121 <undefined>) {
  16:                } -> 0x1a1581304121 <undefined>
  16:                *parseSourceElement+24(this=0x1a1581304121 <undefined>) {
  16:                } -> 0x1a1581304121 <undefined>
  16:                *parseSourceElement+24(this=0x1a1581304121 <undefined>) {
  16:                } -> 0x1a1581304121 <undefined>
  16:                *parseSourceElement+24(this=0x1a1581304121 <undefined>) {
  16:                } -> 0x1a1581304121 <undefined>
  16:                *parseSourceElement+24(this=0x1a1581304121 <undefined>) {
  16:                } -> 0x1a1581304121 <undefined>
  16:                *parseSourceElement+24(this=0x1a1581304121 <undefined>) {
  16:                } -> 0x1a1581304121 <undefined>
  16:                *parseSourceElement+24(this=0x1a1581304121 <undefined>) {
  16:                } -> 0x1a1581304121 <undefined>
  16:                *parseSourceElement+24(this=0x1a1581304121 <undefined>) {
  16:                } -> 0x1a1581304121 <undefined>
  16:                *parseSourceElement+24(this=0x1a1581304121 <undefined>) {
  16:                } -> 0x1a1581304121 <undefined>
  16:                *parseSourceElement+24(this=0x1a1581304121 <undefined>) {
  16:                } -> 0x1a1581304121 <undefined>
  16:                *parseSourceElement+24(this=0x1a1581304121 <undefined>) {
  16:                } -> 0x1a1581304121 <undefined>

@xjamundx
Copy link
Contributor Author

Tracked down the bad code inside of parseSwitchCase() on line 4070:

    while (index < length) {
        if (match("}") || matchKeyword("default") || matchKeyword("case")) {
            break;
        }
        statement = parseSourceElement();
        consequent.push(statement);
    }

If you add console.log(consequent.length) you'll see that consequent grows forever.

@xjamundx
Copy link
Contributor Author

Okay I think something like (if (!statement) { throwErrorTolerant({}, Message.UnclosedSwitch); } may work. Will test and get up a PR on monday. Also will check to see if Esprima has this fixed in their latest.

@nzakas
Copy link
Member

nzakas commented May 30, 2015

Nice analysis! This might be a case where you need to see if lookahead is EOF and just abort.

@nzakas nzakas closed this as completed in 8adbbc8 Jun 1, 2015
nzakas added a commit that referenced this issue Jun 1, 2015
Fix: Incomplete Switch Statement Hangs (Fixes #146)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants