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

Backslashes missing on output code #232

Closed
almosnow opened this issue Jul 2, 2013 · 12 comments
Closed

Backslashes missing on output code #232

almosnow opened this issue Jul 2, 2013 · 12 comments

Comments

@almosnow
Copy link

almosnow commented Jul 2, 2013

I'm using uglify-js inside a node program. I've noticed that sometimes the code returned from an AST is not what I would expect. I don't know if this is a bug or the intended behavior.

How to reproduce:

Execute the following node program:

var uglify = require('uglify-js');
var js = JSON.stringify(['var a = "OK";']);
console.log('A:' + js);
var code = "JSON.parse('" + js + "');";
console.log('B:' + code);
code = uglify.parse(code).print_to_string();
console.log('C: ' + code);

Output:

A: ["var a = \"OK\";"]
B: JSON.parse('["var a = \"OK\";"]');
C: JSON.parse('["var a = "OK";"]');

As you can see, the backslashes are missing on C, this removes the 'escaping' of the quotes surrounding OK and creates an error later if one would try to evaluate the JSON.parse code.

Also, it behaves the same way when explicitly using an OutputStream object.

I'm using uglify-js v2.3.6 and node v0.10.12.

@rvanvelzen
Copy link
Collaborator

This is expected, because \" in a single quotes string is equal to ". See also section 7.8.4 for the specifics of handling escape sequences.

@almosnow
Copy link
Author

almosnow commented Jul 8, 2013

Yeah, well, I don't think those should get escaped.

Check this out:

var uglify = require('uglify-js');
var code = '\'\\" NO WAY, JOSE \\"\'';
console.log('A:' + code);
code = uglify.parse(code).print_to_string();
console.log('B: ' + code);
code = '\'" NO WAY, JOSE "\'';
console.log('C:' + code);
code = uglify.parse(code).print_to_string();
console.log('D: ' + code);

As you will see A != C but B == D, and that is wrong. Sure both strings are actually 'equivalent' (and I can imagine this as one of the reasons why this could've been purposedly coded as a feature), but they are not the same; and you can see in my first example that this situation leads to code that breaks.

@almosnow almosnow closed this as completed Jul 8, 2013
@almosnow almosnow reopened this Jul 8, 2013
@mishoo
Copy link
Owner

mishoo commented Jul 8, 2013

var code = '\'\\" NO WAY, JOSE \\"\'';

After this line, your variable code will contain the following characters:

'\" NO WAY, JOSE \"'

Now you're asking UglifyJS to parse that as code. Copy/paste the above line in a file if you want and run UglifyJS on it, and you'll get the correct result:

'" NO WAY, JOSE "'

Because this thing starts with a quote ('), UglifyJS starts parsing a string. Inside the string, the sequence \" simply means a " character. No bug there.

@mishoo mishoo closed this as completed Jul 8, 2013
@rvanvelzen
Copy link
Collaborator

Inside the string, the sequence " simply means a " character. No bug there.
This was exactly what I was just typing. Funny.

@mishoo
Copy link
Owner

mishoo commented Jul 8, 2013

@rvanvelzen great minds think alike. ;-)

@almosnow
Copy link
Author

almosnow commented Jul 8, 2013

K, got it now, thanks both.

@evolutionxbox
Copy link

evolutionxbox commented Jun 22, 2017

This is still an issue for Regex strings.

new Regex('\d+') will output new Regex('d+')
new RegExp('\d+') will output new RegExp('d+') Edited: 22/06/17 - typo

@rvanvelzen
Copy link
Collaborator

That is not an issue - '\d' === 'd'

@evolutionxbox
Copy link

evolutionxbox commented Jun 22, 2017

That's not true.'\d' !== 'd'

'\d' will match any digit, whereas 'd' will match the character d.

See: https://regex101.com/r/vejuUa/1 and https://regex101.com/r/vejuUa/2

@rvanvelzen
Copy link
Collaborator

We're talking about strings now, not regexes. Just execute the snippet in your console and you'll realize.

@kzc
Copy link
Contributor

kzc commented Jun 22, 2017

$ node
> new RegExp('\d+')
/d+/
> new RegExp('\\d+')
/\d+/
> new Regex('\d+')
ReferenceError: Regex is not defined

@evolutionxbox
Copy link

evolutionxbox commented Jun 22, 2017

Sorry @kzc that's a typo on my part. I did mean RegExp, not Regex.
And yes @rvanvelzen I do now realise...

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

No branches or pull requests

5 participants