Skip to content

Commit

Permalink
Use a reference to Array#concat rather than relying on the runtime …
Browse files Browse the repository at this point in the history 
…environment's `concat`.
  • Loading branch information
@ljharb
ljharb committed May 27, 2015
1 parent 72d43b6 commit 8ce6832
Showing 1 changed file with 3 additions and 2 deletions.
5 changes: 3 additions & 2 deletions es5-shim.js
View file
Expand Up @@ -49,6 +49,7 @@ var array_slice = ArrayPrototype.slice;
var array_splice = ArrayPrototype.splice;
var array_push = ArrayPrototype.push;
var array_unshift = ArrayPrototype.unshift;
var array_concat = ArrayPrototype.concat;
var call = FunctionPrototype.call;

// Having a toString local variable name breaks in Opera so use to_string.
Expand Down Expand Up @@ -235,7 +236,7 @@ defineProperties(FunctionPrototype, {

var result = target.apply(
this,
args.concat(array_slice.call(arguments))
array_concat.call(args, array_slice.call(arguments))
);
if (Object(result) === result) {
return result;
Expand Down Expand Up @@ -264,7 +265,7 @@ defineProperties(FunctionPrototype, {
// equiv: target.call(this, ...boundArgs, ...args)
return target.apply(
that,
args.concat(array_slice.call(arguments))
array_concat.call(args, array_slice.call(arguments))
);

}
Expand Down

8 comments on commit 8ce6832

@bahmutov
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, thanks for making the library a little more robust against unexpected prototype overrides

@raganwald
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, however obviously this only works if the shims are evaluated before any malicious/careless code is evaluated.

@bahmutov
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, I am thinking the websites can control this a little. I see this typical order

  • load es5-shim
  • load trusted libraries from major CDN
  • website's custom code
  • 3rd party code (like ads, etc) that should not be trusted

@ljharb
Copy link
Member Author

@ljharb ljharb commented on 8ce6832 May 28, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's clear in JavaScript that only the first-run script can have any guarantees about security - this is the TC39 committee's position on it, as well.

That loading order is exactly what should be used imo :-)

@bahmutov
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made https://github.com/bahmutov/freeze-prototypes to add precautions after loading trusted libraries and before loading any of the custom application / 3rd party code

@ljharb
Copy link
Member Author

@ljharb ljharb commented on 8ce6832 Jun 1, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's definitely nice and simple. But for security, how is that better than using Caja/SES?

@bahmutov
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caja has so many issues, I am worried about its quality https://github.com/google/caja/issues

@ljharb
Copy link
Member Author

@ljharb ljharb commented on 8ce6832 Jun 1, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issues is an indication of success :-) no issues is an indication that the bugs just haven't been found yet.

Core Caja contributors are on the TC39 committee, and definitely know their stuff. I trust it very much.

Please sign in to comment.