Array.prototype.join must perform ToUint32(length) before ToString(separator)

RESOLVED FIXED in mozilla17

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
2 years ago

People

(Reporter: André Bargull, Assigned: Benjamin)

Tracking

Trunk
mozilla17
x86_64
Windows 7
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:14.0) Gecko/20100101 Firefox/14.0.1
Build ID: 20120713134347

Steps to reproduce:

Per spec, Array.prototype.join [15.4.4.5] performs ToUint32(length) before ToString(separator), cf. step 2/3 and step 5.


var object = {length: {valueOf: function(){ throw "length" }}};
var sep = {toString: function(){ throw "toString" }};
try {
  Array.prototype.join.call(object, sep);
  print("ERROR - expected exception");
} catch (e) {
  if (e !== "length") {
    print("ERROR - expected 'length' but was: " + e);
  } else {
    print("SUCCESS");
  }
}


Actual results:

Output is: `ERROR - expected 'length' but was: toString`


Expected results:

Output should be `SUCCESS`
(Assignee)

Comment 1

5 years ago
Created attachment 646989 [details] [diff] [review]
update join and toLocaleString to follow the spec
Assignee: general → bpeterson
Status: UNCONFIRMED → NEW
Ever confirmed: true
Attachment #646989 - Flags: review?(jorendorff)
Comment on attachment 646989 [details] [diff] [review]
update join and toLocaleString to follow the spec

Review of attachment 646989 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with or without changes.

::: js/src/tests/ecma_5/Array/join-01.js
@@ +59,5 @@
> +var gotLength = false, gotString = false;
> +arr = {length: {valueOf: function () { assertEq(gotString, false); gotLength = true; return 2; }},
> +      0: "x", 1: "z"};
> +var sep = {toString: function () { assertEq(gotLength, true); gotString = true; return "y"; }};
> +assertEq(Array.prototype.join.call(arr, sep), "xyz");

A nice way to test this kind of thing is to keep a log, like this:

var log = '';
arr = {length: {valueOf: function () { log += 'L'; return 2; }}, 0: "x", 1: "z"};
var sep = {toString: function () { log += 'S'; return "y"; }};
assertEq(Array.prototype.join.call(arr, sep), "xyz");
assertEq(log, 'LS');

I think that's a little easier to follow, and it's a stricter test that way. YMMV, just a thought. Same comment in toLocaleString-01.js.
Attachment #646989 - Flags: review?(jorendorff) → review+
(Assignee)

Comment 3

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/a28c47bf240f

Comment 4

5 years ago
https://hg.mozilla.org/mozilla-central/rev/a28c47bf240f
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
(Reporter)

Updated

2 years ago
Duplicate of this bug: 691501
You need to log in before you can comment on or make changes to this bug.