28875 2015-07-01 20:32:39 +0000 [EnforceRange] accepts an odd set of input values 2015-07-02 01:54:06 +0000 1 1 1 Unclassified WebAppsWG WebIDL unspecified PC Linux NEW P2 normal --- 1 jyasskin cam bzbarsky jonas jsbell mike public-script-coord sleevi public-webapps-bugzilla oldest_to_newest 121553 0 jyasskin 2015-07-01 20:32:39 +0000 [EnforceRange] delegates to the ECMAScript ToNumber() operation before checking its range, but this makes it accept a strange set of values. unsigned long enforceRange([EnforceRange] unsigned long arg) { return arg; } enforceRange({}) throws a TypeError, but enforceRange([]) and enforceRange("") return 0. enforceRange(undefined) throws a TypeError, but enforceRange(null) returns 0. What's the "right" behavior? Looking at uses: I see 3 existing uses in Blink, all in IndexedDB: https://code.google.com/p/chromium/codesearch/#search/&q=enforcerange%20file:.idl%20-file:test&type=cs&sq=package:chromium. Web Bluetooth is also considering using [EnforceRange] for BluetoothUUID.canonicalUUID(). 121555 1 bzbarsky 2015-07-01 20:42:53 +0000 > Looking at uses: I see 3 existing uses in Blink, all in IndexedDB: http://mxr.mozilla.org/mozilla-central/search?string=EnforceRange&find=dom%2Fwebidl&findi=&filter=^[^\0]*%24&hitlimit=&tree=mozilla-central shows uses in SubtleCrypto as well. What exactly is the problem with the current behavior? Assuming you plan to do a coercion to number at all, you have to do it before checking range. The only other option would be to immediately throw if the passed-in thing is not a Number value, right? That would break things like passing "123" (e.g. out of input.value) for cases where the callee wants a range-enforced integer. 121557 2 jyasskin 2015-07-01 21:08:59 +0000 Thanks for finding the extra uses. We're excluding some non-integral arguments, but not a consistent set. (That's similar to a lot of Javascript, so maybe it's ok; I'm just checking that it's intentional.) If we want to exclude "strange" input values without breaking "123", I think the appropriate algorithm is something like: 1. Initialize p to ToPrimitive(V, hint Number). 2. If p is "" or null, throw a TypeError. // Excludes empty arrays. 3. Initialize x to ToNumber(p). 4. If x is NaN, +∞, or −∞, then throw a TypeError. 5. Set x to sign(x) * floor(abs(x)). 6. If x < 0 or x > 2^32 − 1, then throw a TypeError. 7. Return the IDL unsigned long value that represents the same numeric value as x. 121563 3 jonas 2015-07-01 22:25:16 +0000 Javascript has a lot of quirks. I think trying to work around those quirks in a random set of DOM APIs is only going to make the platform even more quirky. What's special about [EnforceRange] here? It seems like you'd see the same set of quirky behavior for any API that accepts a number. Consider for example x = [1, 2]; x.length = 0; // Removes all items for x x.length = {}; // Throws and leaves x unaffected x.length = []; // Removes all items for x x.length = ""; // Removes all items for x 121565 4 jyasskin 2015-07-01 23:04:29 +0000 [EnforceRange] already works around a quirk of Javascript in a random set of (not primarily DOM) APIs. If it's going to exist at all, it may as well work around the quirk well. Most APIs that take integer types uniformly treat all of {}, [], undefined, null, and NaN as 0. Only APIs that try to enforce an integral range pick an arbitrary subset of those to throw an exception on. array.length is a good precedent for how to enforce ranges, but http://ecma-international.org/ecma-262/6.0/#sec-arraysetlength already behaves differently from [EnforceRange]. It checks that ToNumber(arg) and ToUint32(arg) give the same result, which throws a TypeError on "x.length = 3.2". enforceRange(3.2) returns 3. Maybe we should just unify those behaviors and call it a day? It wouldn't be quite the ideal behavior, but it'd be consistently odd. 121566 5 jonas 2015-07-01 23:48:45 +0000 I'd definitely go for consistency rather than trying to fix JS quirks. 121569 6 bzbarsky 2015-07-02 01:54:06 +0000 > but not a consistent set Well, it's consistent with how everything else in JS converts things to numbers... It's fairly intentionally matching those other conversions, yes. You raise a good point about how conversion to integers in JS typically converts non-finite doubles to 0. That's a pretty odd thing to do in [EnforceRange], though (at least for the positive and negative Infinity values). So [EnforceRange] basically just codifies what people would otherwise do in prose: take a double or 53-bit int (depending on whether they wanted to throw or round on non-integers). and then check the range. > Maybe we should just unify those behaviors I could live with that, probably; we'd need to think a bit about what the behavior looks like for integer sizes other than 32-bit.