Skip to main content

Bug Tracker

Side navigation

#13714 closed bug (fixed)

Opened April 02, 2013 08:09PM UTC

Closed April 03, 2013 03:26PM UTC

Last modified April 04, 2013 12:23PM UTC

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.

Attachments (0)
Change History (22)

Changed April 02, 2013 08:29PM UTC by dmethvin comment:1

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?

Changed April 02, 2013 08:33PM UTC by jdalton comment:2

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.

Changed April 02, 2013 08:36PM UTC by jdalton comment:3

@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

Changed April 02, 2013 08:45PM UTC by jaubourg comment:4

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?

Changed April 02, 2013 08:53PM UTC by anonymous comment:5

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

Changed April 02, 2013 08:54PM UTC by jdalton comment:6

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

Changed April 02, 2013 09:02PM UTC by jaubourg comment:7

Replying to [comment:6 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 ;)

Changed April 02, 2013 09:03PM UTC by jaubourg comment:8

Replying to [comment:5 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.

Changed April 02, 2013 10:22PM UTC by jdalton comment:9

@jaubourg Naw, I like that ES5 eval code has this feature.

I don't think its a spec bug at all.

Changed April 02, 2013 10:27PM UTC by jdalton comment:10

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

Changed April 02, 2013 10:51PM UTC by jaubourg comment:11

Replying to [comment:9 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?

Changed April 02, 2013 10:53PM UTC by jaubourg comment:12

Replying to [comment:10 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 ;)

Changed April 02, 2013 10:57PM UTC by rwaldron comment:13

_comment0: Replying to [comment:4 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 eval code with "use strict" is the only sane semantics \ \ \ Are we talking about changes in jQuery 2.x or 1.x?1364943540890221

Replying to [comment:4 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?

Changed April 02, 2013 11:03PM UTC by jaubourg comment:14

Replying to [comment:13 rwaldron]:

Replying to [comment:4 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?

Changed April 02, 2013 11:24PM UTC by rwaldron comment:15

Replying to [comment:14 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

}());

Changed April 02, 2013 11:26PM UTC by rwaldron comment:16

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

Changed April 02, 2013 11:49PM UTC by jaubourg comment:17

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?

Changed April 03, 2013 12:46AM UTC by m_gol comment:18

_comment0: @ 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 make the following 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...1364950021815642

@ 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...

Changed April 03, 2013 10:53AM UTC by jaubourg comment:19

owner: → 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?

Changed April 03, 2013 02:42PM UTC by jdalton comment:20

_comment0: @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 \ }}}1365000154064442
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
// => global foo: 1 

Changed April 03, 2013 03:26PM UTC by Rick Waldron comment:21

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@gmail.com>

Changeset: feea9394b75a5dcb16fad013a402b388f97cefba

Changed April 04, 2013 12:23PM UTC by jaubourg comment:22

Replying to [comment:20 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 :/