google / traceur-compiler

Traceur is a JavaScript.next-to-JavaScript-of-today compiler
Apache License 2.0
8.17k stars 580 forks source link

Small compliance issues in the new String polyfills #562

Open anba opened 10 years ago

anba commented 10 years ago

The new String polyfills are not completely spec compliant for certain edge cases.

Two issues in String.prototype.startsWith:

One issue in String.prototype.endsWith:

One issue in String.prototype.contains:

Three issues in String.raw:


Simple test cases for each issue:

String.prototype.startsWith.call({toString(){ throw 0 }}, /./);

Expected: throws 0 Actual: throws TypeError

Object.prototype[1] = 2;
"ab".startsWith("ab");

Expected: returns true Actual: returns false

String.prototype.endsWith.call({toString(){ throw 0 }}, /./);

Expected: throws 0 Actual: throws TypeError

Object.prototype[1] = 2;
"ab".contains("ab")

Expected: returns true Actual: returns false

Object.defineProperty(Boolean.prototype, "length", {
  get() {
    "use strict";
    if (typeof this == 'boolean') throw new Error("missing ToObject");
    return 0;
  }, configurable: true
});
String.raw({raw:true});

Expected: "" Actual: throws Error("missing ToObject")

String.raw({raw: {length: -1}})

Expected: returns "" Actual: most likely causes an out-of-memory error in browsers...

Array.prototype[0] = "!";
String.raw({raw:"ab".split("")});

Expected: "a!b" Actual: "aundefinedb"

arv commented 10 years ago

@mathiasbynens FYI

arv commented 10 years ago

Thanks for the detailed report.

mathiasbynens commented 10 years ago

@anba Awesome feedback, as usual :) I’ll get these fixed. Thanks!

anba commented 10 years ago

You're welcome! And since I'm re-using parts of the traceur test suite for my own project, it's only fair to file bug reports every now and then. ^^

mathiasbynens commented 10 years ago

@anba Actually, the tests for the first three bugs you reported were taken from http://mths.be/startswith, http://mths.be/endswith, and http://mths.be/contains. I just happened to contribute them to Traceur :)

I’ve fixed the issues you mentioned in those three projects and will be submitting patches to Traceur as soon as possible.

FWIW, in environments where Object.prototype.toString or String.prototype.indexOf is overridden, most of these polyfills will fail too. Not sure if that’s something we need to guard against. @arv, any thoughts?

arv commented 10 years ago

@mathiasbynens I was considering creating a module that captures these as early as possible.