Bug Tracker

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#13714 closed bug (fixed)

jQuery.globalEval gotcha with strings that contain the "use strict" directive.

Reported by: jdalton Owned by: jdalton
Priority: undecided Milestone: None
Component: unfiled Version: 1.9.1
Keywords: Cc:
Blocked by: Blocking:

Description

jQuery.globalEval relies on indirect eval in modern browsers, but when text contains the "use strict" directive (http://es5.github.com/#x10.4.2; step 3; sub step 1), it's executed as if it's wrapped with an IIFE

(function() {
  
  var code = [
    '"use strict";',
    'var foo = 1;',
    'console.log("inside eval window.foo: " + window.foo)'
  ].join('\n');
      
  var jqCode = [
    '"use strict";',
    'var foo = 1;',
    'console.log("inside jQuery.globalEval window.foo: " + window.foo)'
  ].join('\n');
  
  (0, eval)(code);
  jQuery.globalEval(jqCode);
  console.log('local foo: ' + (typeof foo != 'undefined' ? foo : undefined));
}());

console.log('global foo: ' + window.foo);

// => "inside eval window.foo: undefined"
// => "inside jQuery.globalEval window.foo: undefined"
// => "local foo: undefined"
// => "global foo: undefined"

See http://jsbin.com/ujedax/1 (open the console)

The solution would be to attempt to use script injection instead.

Change History (22)

comment:1 Changed 6 years ago by dmethvin

I wouldn't be so fast to switch approaches entirely, we'll just exchange for a different set of quirks.

When/how does this impact real code?

comment:2 Changed 6 years ago by jdalton

Actually, script injection avoids the need to use execScript in old IE, and the extra parsing it requires too. The downside is that it requires the DOM.

comment:3 Changed 6 years ago by jdalton

@kangax points out that instead of switching to script injection you can continue to use the indirect eval and simple prefix something like 1; to the data string so that it's no longer a valid directive: https://twitter.com/kangax/status/319185222157619200

comment:4 Changed 6 years ago by jaubourg

The downside is also that it doesn't guarantee synchronous execution. Unless we can have definite proof this is no longer a problem?

Don't get me wrong, I'd love to switch back to script tag injection (so much less code and regexps and, now, insane "1;" prefixes and whatnot to act like a normal script tag obviously) but I know why we switched to the current eval/execScript madness.

Now ES5 eval acting differently if the code is strict doesn't make a lot of sense to me: the same code, executed in a script tag would have the vars declared globally, so why on earth is the standard asking for eval to change its semantics? Am I missing something huge here? Isn't it a bug in the standard?

comment:5 Changed 6 years ago by anonymous

Actually killing the "use strict" directive with 1; will change the way the code works so may not be the best approach /via @abozhilov

comment:6 Changed 6 years ago by jdalton

btw anonymous was me (switched browsers, forgot to sign in)

comment:7 in reply to:  6 Changed 6 years ago by jaubourg

Replying to jdalton:

btw anonymous was me (switched browsers, forgot to sign in)

Having an icon that pops a browser at random suits you so well when you think about it ;)

comment:8 in reply to:  5 Changed 6 years ago by jaubourg

Replying to anonymous:

Actually killing the "use strict" directive with 1; will change the way the code works so may not be the best approach /via @abozhilov

I also agree with @abozhilov this is a problem in the spec.

comment:9 Changed 6 years ago by jdalton

@jaubourg Naw, I like that ES5 eval code has this feature. I don't think its a spec bug at all.

comment:10 Changed 6 years ago by jdalton

For reference here is an earlier ticket related to the Firefox < 4 issue: http://bugs.jquery.com/ticket/7862

Following jQuery's support page it should now be OK to use script injection again: http://jquery.com/browser-support/

Here is the GitHub thread following the commit to change from script injection to the current technique: https://github.com/jquery/jquery/commit/f3c6077da02f080f09d73ec4d8a8029f76654c2b

comment:11 in reply to:  9 Changed 6 years ago by jaubourg

Replying to jdalton:

@jaubourg Naw, I like that ES5 eval code has this feature. I don't think its a spec bug at all.

Why? What could possibly be the use of this when you can just wrap code in a closure manually? Why could be the use of this except to effectively render eval useless?

comment:12 in reply to:  10 Changed 6 years ago by jaubourg

Replying to jdalton:

For reference here is an earlier ticket related to the Firefox < 4 issue: http://bugs.jquery.com/ticket/7862

Following jQuery's support page it should now be OK to use script injection again: http://jquery.com/browser-support/

Here is the GitHub thread following the commit to change from script injection to the current technique: https://github.com/jquery/jquery/commit/f3c6077da02f080f09d73ec4d8a8029f76654c2b

Yeah, it's probably time but I'd be much more re-assured by a FF bug tracker ticket that clearly states this is fixed.

Gotta create a unit test for this ;)

comment:13 Changed 6 years ago by Rick Waldron

Replying to jaubourg:

Now ES5 eval acting differently if the code is strict doesn't make a lot of sense to me: the same code, executed in a script tag would have the vars declared globally, so why on earth is the standard asking for eval to change its semantics? Am I missing something huge here? Isn't it a bug in the standard?

No bug. Strict mode must ensure static scoping semantics, which includes scope determinism. Specifically, global scope isn't implied the same way it is in non-strict mode. Also, if you could eval "strict mode" code in the global scope, should the the global environment record's strict flag then be set to true? I hope we all answered "no" to that question, since the "strict-ness" is static, not dynamic. Creating a fresh environment record to execute global eval code with "use strict" is the only sane semantics

Are we talking about changes in jQuery 2.x or 1.x?

Last edited 6 years ago by Rick Waldron (previous) (diff)

comment:14 in reply to:  13 ; Changed 6 years ago by jaubourg

Replying to rwaldron:

Replying to jaubourg:

Now ES5 eval acting differently if the code is strict doesn't make a lot of sense to me: the same code, executed in a script tag would have the vars declared globally, so why on earth is the standard asking for eval to change its semantics? Am I missing something huge here? Isn't it a bug in the standard?

No bug. Strict mode must ensure static scoping semantics, which includes scope determinism. Specifically, global scope isn't implied the same way it is in non-strict mode. Also, if you could eval "strict mode" code in the global scope, should the the global environment record's strict flag then be set to true? I hope we all answered "no" to that question, since the "strict-ness" is static, not dynamic. Creating a fresh environment record to execute global eval code with "use strict" is the only sane semantics

Are we talking about changes in jQuery 2.x or 1.x?

It's probably safe for both by now.

If I'm getting what you're saying correctly, then what is the semantics of inserting an inline script tag with strict code? Is this even specified btw?

comment:15 in reply to:  14 ; Changed 6 years ago by Rick Waldron

Replying to jaubourg:

Are we talking about changes in jQuery 2.x or 1.x?

It's probably safe for both by now.

What is "probably safe" by now?

If I'm getting what you're saying correctly, then what is the semantics of inserting an inline script tag with strict code? Is this even specified btw?

First, run this:

function foo() {
  var arguments = 1;
}

foo(); // doesn't throw

Then this:

var script = document.createElement("script");
script.textContent = "'use strict'; console.log(1);";
document.head.appendChild(script);

Then this:

foo(); // doesn't throw

Object.defineProperty(window, "c", { value: 1 });

c = 2; // doesn't throw

The global environment record won't become "strict" this way

Compare to an explicit, static strict mode opt-in:

(function() {
  "use strict";

  function foo() {
    var arguments = 1;
  }

  foo(); // throws

  // It won't get to here, because var arguments = 1; is a syntax error
  // but if you comment out the above and re-run with just this....

  Object.defineProperty(window, "c", { value: 1 });

  c = 2; // throws

}());

comment:16 Changed 6 years ago by Rick Waldron

Also, the script element is not the business of the ECMAScript specification

comment:17 in reply to:  15 Changed 6 years ago by jaubourg

What is "probably safe" by now?

To revert back to script tag injection.

Also, I'm getting the fact the global env won't become "strict" because of a single injected script tag. But if the following actually executes in strict mode, at the global level:

var script = document.createElement("script");
script.textContent = "'use strict'; console.log(1);";
document.head.appendChild(script);

Then I fail to see why eval does act differently.

Unless, of course, a script tag injected that way simply ignores the "use strict"; directive but that doesn't seem to be the case.

This is the crux of my interrogation here. I'm well aware there are two separate standard bodies here but I'd expect them both to thrive for consistency, not take completely opposite decisions on executing script from source.

How can the problem of scope determinism you talk about incur such a constraint on eval, when the issue is pretty much ignored by the number one js env when using inline script tag injection? Which is wrong, if any?

comment:18 Changed 6 years ago by m_gol

@ jaubourg eval behavior in this case can't be changed due to what rwaldron said. But how would you want the script injection method to behave in the same way? The only method I see to achieve that would be to require the following to throw:

<script>'use strict'; var a = 2;</script>
<script>'use strict'; var b = a + 1;</script>

This is because you can't detect if <script> tags have been present since the beginning or if one of them was injected using JavaScript code.

On the other hand, not enclosing the 'use strict'; directive inside a function is considered a bad idea anyway...

Last edited 6 years ago by m_gol (previous) (diff)

comment:19 Changed 6 years ago by jaubourg

Owner: set to jdalton
Status: newpending

Hmmm, in Chrome, script tag injection acts like eval actually: http://jsbin.com/ujedax/2/, @jdalton, did you actually test this with script tag injection? On which browsers?

comment:20 Changed 6 years ago by jdalton

Status: pendingnew

@jaubourg Yes, I tested it before I reported the issue. You should try script injection in a more basic form before attempting to do it with jQuery. See http://jsbin.com/ewukuh/1.

// => inside eval window.foo: undefined
// => inside jQuery.globalEval window.foo: undefined
// => inside script tag injection window.foo: 1 1:3
// => global foo: 1 
Version 0, edited 6 years ago by jdalton (next)

comment:21 Changed 6 years ago by Rick Waldron

Resolution: fixed
Status: newclosed

Fixes #13714. jQuery.globalEval gotcha w/ strings that contain valid, prologue position strict mode pragma

Signed-off-by: Rick Waldron <waldron.rick@…>

Changeset: feea9394b75a5dcb16fad013a402b388f97cefba

comment:22 in reply to:  20 Changed 6 years ago by jaubourg

Replying to jdalton:

@jaubourg Yes, I tested it before I reported the issue. You should try script injection in a more basic form before attempting to do it with jQuery. See http://jsbin.com/ewukuh/1.

// => inside eval window.foo: undefined
// => inside jQuery.globalEval window.foo: undefined
// => inside script tag injection window.foo: 1
// => global foo: 1 

And that's the silly frenchie owned by jQuery once again :/

Note: See TracTickets for help on using tickets.