Skip Menu |

This queue is for tickets about the JavaScript-Minifier-XS CPAN distribution.

Report information
The Basics
Id: 51008
Status: resolved
Priority: 0/
Queue: JavaScript-Minifier-XS

People
Owner: Nobody in particular
Requestors: james [...] neodon.com
Cc:
AdminCc:

Bug Information
Severity: Normal
Broken in: 0.06
Fixed in: 0.10



Subject: regex can cause 'unterminated quoted string literal' error
Hi, This module is awesome. I'd like to let you know about an issue I'm having when minifying the Tablesorter plugin for jQuery. Download: http://tablesorter.com/jquery.tablesorter.zip File: tablesorter/jquery.tablesorter.js Line: 796 return /\d{1,2}[\/\-]\d{1,2}[\/\-]\d{2,4}/.test(s); This line causes the minify function to croak with the message "unterminated quoted string literal". I was able to reproduce the failure with these cases: perl -MJavaScript::Minifier::XS=minify -e 'minify(q{return /\d{1,2}[\/\-]\d{1,2}[\/\-]\d{2,4}/.test(s);})' perl -MJavaScript::Minifier::XS=minify -e 'minify(q{return /\d{1,2}[\/\-]\d{1,2}[\/\-]\d{2,4}/;})' perl -MJavaScript::Minifier::XS=minify -e 'minify(q{return /\d+/;})' After reading the XS code and trying some more test cases, I think I can narrow down the problem a little. It appears to involve this condition in JsTokenizeString(): /* see if we're "division" or "regexp" */ if (ch && ((ch == ')') || (ch == '.') || (ch == ']') || (charIsIdentifier(ch)))) _JsExtractSigil(&doc, node); /* division */ else _JsExtractLiteral(&doc, node); /* regexp */ It looks like the first '/' delimiter for the regex is being interpreted as division because the 'return' keyword matches an identifier character and satisfies the condition. Replacing 'return' in front of the regex with a character that doesn't match the first condition prevents the error message, and replacing 'return' with any other character that would match the first condition causes the error message to appear. I'm interested to know if you are able to reproduce this problem or if it could be something wrong with my configuration. Let me know if you need any more information.
Sorry about the pointless status change. Here is my work-around for now that changes the JavaScript before minifying it: $js =~ s{return \s+ /}{return true && /}gxms;
On Sat Oct 31 14:08:39 2009, neodon wrote: Show quoted text
> Sorry about the pointless status change. > > Here is my work-around for now that changes the JavaScript before > minifying it: > > $js =~ s{return \s+ /}{return true && /}gxms;
I believe the problem is here in XS.xs: /* see if we're "division" or "regexp" */ if (ch && ((ch == ')') || (ch == '.') || (ch == ']') || (charIsIdentifier(ch)))) _JsExtractSigil(&doc, node); /* division */ else _JsExtractLiteral(&doc, node); /* regexp */ The scanner sees that the last character before the expression is a "literal" (the n in return), so it assumes it's dealing with a division, when actually it should be expecting a regular expression.
On Sat Oct 31 14:08:39 2009, neodon wrote: Show quoted text
> Sorry about the pointless status change. > > Here is my work-around for now that changes the JavaScript before > minifying it: > > $js =~ s{return \s+ /}{return true && /}gxms;
I believe the problem is here in XS.xs: /* see if we're "division" or "regexp" */ if (ch && ((ch == ')') || (ch == '.') || (ch == ']') || (charIsIdentifier(ch)))) _JsExtractSigil(&doc, node); /* division */ else _JsExtractLiteral(&doc, node); /* regexp */ The scanner sees that the last character before the expression is a "literal" (the n in return), so it assumes it's dealing with a division, when actually it should be expecting a regular expression.
if ( 0 != strncmp( last->content, "return", 6 ) && chr && ( ( chr == ')' ) || ( chr == '.' ) || ( chr == ']' ) || ( is_identifier( chr ) ) ) ) /* A division */ else /* A regular expression */ Here is a (potential) hacky fix: check to see last node is return or not
I just stumbled upon this issue today, coincidentally on jquery.tablesorter. I appreciated finding this ticket that described the problem. I ended up adding parens around the return value in my local copy which worked around the issue.