1275183 - Hang if Intl.getCanonicalLocales gets an Array-like-object with negative length

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Zibi Braniecki [:zbraniecki][:gandalf]]

This code hangs SpiderMonkey:

```

var x = {}; 
Object.defineProperty(x, 'length', {get: function(){return -5}});

Intl.getCanonicalLocales(x);

```

Same happens with any of the Intl formatters if you pass x as the first param, because it's the same operation.

Seems like something is going wrong with https://dxr.mozilla.org/mozilla-central/source/js/src/builtin/Intl.js#664

:waldo, any ideas?

Comment 1

[Jeff Walden [:Waldo]]

To be clear, when you say "hang", you mean in the shell only, right?  And if you try in a browser, you get a slow script dialog?  If the latter doesn't happen we consider that a bug, but we don't care about the former.  A script that runs forever *should* offer the opportunity to cancel in the browser, but it *should* keep running as long as the user allows it to.

As for the actual issue here.

Prior versions of the spec did ToUint32(O.length), to determine how many iterations to run grabbing locales off the locale-list value.  That would convert -1 to uint32_t(-1), i.e. a big value.  Latest spec iteration, on the other hand, seems to use ToLength instead -- which says, "If len ≤ +0, return +0."

So the fix is approximately s/TO_UINT32/ToLength/ here (plus a test), as long as ToLength is defined for self-hosting code use.  I think it is, but you'll have to double-check.

Note that a better form of the test would be this (so the test fails fast, rather than taking forever and a day to fail by timing out):

  var obj = { get 0() { throw new Error("must not be gotten!"); },
              length: -Math.pow(2, 32) + 1 };

  // With older ECMA-402 editions, TO_UINT32(obj.length) would be 1, so obj[0]
  // would be accessed.  But newer editions' ToLength(obj.length) is +0, so
  // obj[0] must not be accessed.
  Intl.getCanonicalLocales(obj);

Comment 2

[Zibi Braniecki [:zbraniecki][:gandalf]]

Cool! It did fix the issue.

I'm about to land test262 tests for getCanonicalLocales which will cover those scenarios. Do you want me to also add local tests for this with the fix, or is test262 coverage enough?

Comment 3

[Jeff Walden [:Waldo]]

Add my seven-liner test in both places.  Eventually we'll have test262 importation be turnkey, but there's tc39-side coordination to do first, and we want the test in advance of that.

Comment 4

[Zibi Braniecki [:zbraniecki][:gandalf]]

Attachment: bug1275183.diff

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0a2fe25f4c67
"Hang if Intl.getCanonicalLocales gets an Array-like-object with negative length". r=waldo

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/0a2fe25f4c67