Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 433709

Summary: [esprima] parser gets into infinite loop
Product: [ECD] Orion Reporter: Mark Macdonald <mamacdon>
Component: JS ToolsAssignee: Michael Rennie <Michael_Rennie>
Status: RESOLVED FIXED QA Contact:
Severity: critical    
Priority: P3 CC: Michael_Rennie
Version: 6.0   
Target Milestone: 6.0 M2   
Hardware: PC   
OS: Windows 7   
Whiteboard:
Attachments:
Description Flags
screenshot none

Description Mark Macdonald CLA 2014-04-28 16:36:41 EDT
1. Enable mark occurrences
2. Create a file named 'test.html' containing this content:
<!DOCTYPE html>
<html>
<head>
	<script>
	require({
		baseUrl: '../../../',
		paths: {
			foo/bar": "foo/bar"   // note missing " in obj key
		}
	});
	</script>
</head>
<body>
</body>
</html>

3. Try to save the file.
4. Browser hangs.

If you turn on exception breakpoints, you can see the parser encounters an error at index==74, "Unexpected token ILLEGAL". It tries to recover, but index is never incremented. It just keeps hitting the same error again and again.

It is also possible to get into this state without saving, if mark occurrences happens to run.
Comment 1 Mark Macdonald CLA 2014-04-28 16:57:34 EDT
Oops. Step #1 is not required.
Comment 2 Michael Rennie CLA 2014-04-28 18:58:39 EDT
Created attachment 242432 [details]
screenshot

I am not seeing the hanging. Are you running on the latest from master?

Using your sample code I get what is shown in the screen shot (the object expression is recovered and the 'foo' of foo/bar" is marked)

Debugging the new parser I found I (correctly) get exactly three calls to #parseObjectProperty - I made sure to do the localStorage / history delete / refresh dance first.
Comment 3 Michael Rennie CLA 2014-04-29 10:24:25 EDT
I added some new unit tests which exercise the code reported to cause the hang:
http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=e3c8b29145d86e54bb13aaf50be22d33325ef9a4

they all pass FWIW.
Comment 4 Michael Rennie CLA 2014-04-29 10:52:57 EDT
Mark provided me with a target workspace that produced the hang, and here is the stack trace:

parseExpression (esprima.js:2667)
(anonymous function) (esprima.js:4023)
parseStatement (esprima.js:3349)
(anonymous function) (esprima.js:4038)
parseSourceElement (esprima.js:3594)
parseSourceElements (esprima.js:3627)
parseProgram (esprima.js:3643)
parse (esprima.js:3912)
Objects.mixin.parse (astManager.js:70)
(anonymous function) (astManager.js:58)
settleDeferred (Deferred.js:71)
notify (Deferred.js:145)
run (Deferred.js:29)

The problem is we get into a state where the lookahead token is not advanced and we get an exception parsing (expected) so we infinitely keep trying to parse the same token. In the given example it is the last 'bar' token
Comment 5 Michael Rennie CLA 2014-04-29 13:42:26 EDT
The problem boils down to an incomplete string literal that we are trying to recover from during the object recovery. Any calls to peek / lex / advance / collectToken all fail because the function scanStringLiteral throws an exception when it does not find a closing quote. This happens each iteration, with the same token, over and over and over again.

The simple solution is to change the call in scanStringLiteral from throwError to throwErrorTolerant, allowing the opening quote to be recovered, which in turn allow the index to be incremented. 

If we are going to support literal recovery we should probably add a pile of tests and some more heuristics like:

1. we can stop recovering when we hit the end of the line and have found no closing quote (\r\n / \n)
2. we an produce a useful error when in the recovery state like 'Unclosed string literal' (or similar) and tag the opening quote - rather than marking the entire unclosed literal with the error 'Unexpected token ILLEGAL'