binjs-ref icon indicating copy to clipboard operation
binjs-ref copied to clipboard

Stack overflow parsing shift AST of lots of string concatenation

Open dominiccooney opened this issue 5 years ago • 7 comments

Here's a 52 KB/700 line file file I encountered in a random sample of httparchive: f724.js.zip

The nature of the file is:

window["fpdefs"] = {
vertex_shader :
'attribute vec3 a_vertexPosition,a_vertexNormal;attribute vec2 a_vert' +
'exTexture;uniform mat4 u_mvMatrix,u_pMatrix,u_nMatrix;uniform vec3 u' +
...
texture :
'/9j/4AAQSkZJRgABAQEAAQABAAD/2wBDABALDA4MChAODQ4SERATGCgaGBYWGDEjJR0o' +
'OjM9PDkzODdASFxOQERXRTc4UG1RV19iZ2hnPk1xeXBkeFxlZ2P/wAALCAIAAgABAREA' +
...

This caused a stack overflow building dictionaries with d477a474500849c9c26abc51de27e7a399f5b77b:

Treating "/.../httparchive/chrome-Nov_1_2017/f724.js" ("")
Parsing.
thread 'large stack dedicated thread' panicked at 'Could not parse source: JsonError(ExceededDepthLimit)', libcore/result.rs:1009:5
note: Run with `RUST_BACKTRACE=1` for a backtrace.
thread 'main' panicked at 'Error in dedicated thread: Any', libcore/result.rs:1009:5

I guess when the Shift AST parser goes away, so will this JSON → AST step. Do you have a pointer to the Rust JavaScript parser you want to replace Shift with?

dominiccooney avatar Nov 26 '18 00:11 dominiccooney

As discussed somewhere else, one possible idea is to replace this with ratel.

Yoric avatar Jan 18 '19 06:01 Yoric

See #229 (inactive atm).

Yoric avatar Jan 18 '19 06:01 Yoric

I can confirm that the problem still appears as of tag entropy-0.2.0.

Yoric avatar Jan 29 '19 18:01 Yoric

Cc @RReverser that's a demonstration of the kind of issues we encounter with JSON parsing in real examples already.

Yoric avatar Jan 29 '19 22:01 Yoric

Understood. @dominiccooney Note however that replacing with another parser won't alleviate the problem, it can just increase the limit at which a fatal stack overflow occurs.

RReverser avatar Jan 30 '19 17:01 RReverser

@RReverser Yeah I understand that. FWIW I don't think Ratel tip has been able to parse fb_bench for a while. (Maybe I'm holding it wrong.)

dominiccooney avatar Feb 01 '19 05:02 dominiccooney

@dominiccooney When you're talking about fb_bench, are you talking about this snapshot or anything more recent?

Yoric avatar Feb 01 '19 08:02 Yoric