Skip to content

Commit

Permalink
Don't coerce non-String inputs to strings
Browse files Browse the repository at this point in the history
This commit stops converting non-string inputs to strings when
parsing ISO strings or property bag fields.

When numbers are coerced to strings, the result is sometimes a valid
ISO string subset but it often doesn't behave as expected. The result is
often brittle, yielding "data driven exceptions" that we've tried to
avoid.

Numbers can also be ambiguous, e.g. is a Number passed for
`offset` mean hours like an ISO string, minutes like
Date.p.getTimezoneOffset, or msecs like Date.p.getTime().

This commit also removes coercing objects to strings in the
same contexts (like TimeZone and Calendar constructors) because
it's unclear that there are use cases for this coercion.
  • Loading branch information
justingrant authored and ptomato committed Jul 18, 2023
1 parent c429eed commit 7723794
Show file tree
Hide file tree
Showing 15 changed files with 86 additions and 69 deletions.
14 changes: 4 additions & 10 deletions polyfill/lib/calendar.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -41,20 +41,14 @@ const impl = {};

export class Calendar {
constructor(id) {
// Note: if the argument is not passed, IsBuiltinCalendar("undefined") will fail. This check
// exists only to improve the error message.
if (arguments.length < 1) {
throw new RangeError('missing argument: id is required');
}

id = ES.ToString(id);
if (!ES.IsBuiltinCalendar(id)) throw new RangeError(`invalid calendar identifier ${id}`);
const stringId = ES.RequireString(id);
if (!ES.IsBuiltinCalendar(stringId)) throw new RangeError(`invalid calendar identifier ${stringId}`);
CreateSlots(this);
SetSlot(this, CALENDAR_ID, ES.ASCIILowercase(id));
SetSlot(this, CALENDAR_ID, ES.ASCIILowercase(stringId));

if (typeof __debug__ !== 'undefined' && __debug__) {
Object.defineProperty(this, '_repr_', {
value: `${this[Symbol.toStringTag]} <${id}>`,
value: `${this[Symbol.toStringTag]} <${stringId}>`,
writable: false,
enumerable: false,
configurable: false
Expand Down
47 changes: 33 additions & 14 deletions polyfill/lib/ecmascript.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ const ObjectCreate = Object.create;
const ObjectDefineProperty = Object.defineProperty;
const ObjectEntries = Object.entries;
const ObjectGetOwnPropertyDescriptor = Object.getOwnPropertyDescriptor;
const StringCtor = String;
const StringFromCharCode = String.fromCharCode;
const StringPrototypeCharCodeAt = String.prototype.charCodeAt;
const StringPrototypeMatchAll = String.prototype.matchAll;
Expand Down Expand Up @@ -147,10 +148,27 @@ export function ToIntegerIfIntegral(value) {
return number;
}

// This convenience function isn't in the spec, but is useful in the polyfill
// for DRY and better error messages.
export function RequireString(value) {
if (Type(value) !== 'String') {
// Use String() to ensure that Symbols won't throw
throw new TypeError(`expected a string, not ${StringCtor(value)}`);
}
return value;
}

// This function is an enum in the spec, but it's helpful to make it a
// function in the polyfill.
function ToPrimitiveAndRequireString(value) {
value = ToPrimitive(value, StringCtor);
return RequireString(value);
}

const BUILTIN_CASTS = new Map([
['year', ToIntegerWithTruncation],
['month', ToPositiveIntegerWithTruncation],
['monthCode', ToString],
['monthCode', ToPrimitiveAndRequireString],
['day', ToPositiveIntegerWithTruncation],
['hour', ToIntegerWithTruncation],
['minute', ToIntegerWithTruncation],
Expand All @@ -168,9 +186,9 @@ const BUILTIN_CASTS = new Map([
['milliseconds', ToIntegerIfIntegral],
['microseconds', ToIntegerIfIntegral],
['nanoseconds', ToIntegerIfIntegral],
['era', ToString],
['era', ToPrimitiveAndRequireString],
['eraYear', ToIntegerOrInfinity],
['offset', ToString]
['offset', ToPrimitiveAndRequireString]
]);

const BUILTIN_DEFAULTS = new Map([
Expand Down Expand Up @@ -699,7 +717,7 @@ export function RegulateISOYearMonth(year, month, overflow) {

export function ToTemporalDurationRecord(item) {
if (Type(item) !== 'Object') {
return ParseTemporalDurationString(ToString(item));
return ParseTemporalDurationString(RequireString(item));
}
if (IsTemporalDuration(item)) {
return {
Expand Down Expand Up @@ -981,7 +999,7 @@ export function ToRelativeTemporalObject(options) {
} else {
let tzName, z;
({ year, month, day, hour, minute, second, millisecond, microsecond, nanosecond, calendar, tzName, offset, z } =
ParseISODateTime(ToString(relativeTo)));
ParseISODateTime(RequireString(relativeTo)));
if (tzName) {
timeZone = ToTemporalTimeZoneSlotValue(tzName);
if (z) {
Expand Down Expand Up @@ -1139,7 +1157,7 @@ export function ToTemporalDate(item, options) {
return CalendarDateFromFields(calendar, fields, options);
}
ToTemporalOverflow(options); // validate and ignore
let { year, month, day, calendar, z } = ParseTemporalDateString(ToString(item));
let { year, month, day, calendar, z } = ParseTemporalDateString(RequireString(item));
if (z) throw new RangeError('Z designator not supported for PlainDate');
if (!calendar) calendar = 'iso8601';
if (!IsBuiltinCalendar(calendar)) throw new RangeError(`invalid calendar identifier ${calendar}`);
Expand Down Expand Up @@ -1213,7 +1231,7 @@ export function ToTemporalDateTime(item, options) {
ToTemporalOverflow(options); // validate and ignore
let z;
({ year, month, day, hour, minute, second, millisecond, microsecond, nanosecond, calendar, z } =
ParseTemporalDateTimeString(ToString(item)));
ParseTemporalDateTimeString(RequireString(item)));
if (z) throw new RangeError('Z designator not supported for PlainDateTime');
RejectDateTime(year, month, day, hour, minute, second, millisecond, microsecond, nanosecond);
if (!calendar) calendar = 'iso8601';
Expand Down Expand Up @@ -1248,7 +1266,8 @@ export function ToTemporalInstant(item) {
const TemporalInstant = GetIntrinsic('%Temporal.Instant%');
return new TemporalInstant(GetSlot(item, EPOCHNANOSECONDS));
}
const ns = ParseTemporalInstant(ToString(item));
item = ToPrimitive(item, StringCtor);
const ns = ParseTemporalInstant(RequireString(item));
const TemporalInstant = GetIntrinsic('%Temporal.Instant%');
return new TemporalInstant(ns);
}
Expand Down Expand Up @@ -1278,7 +1297,7 @@ export function ToTemporalMonthDay(item, options) {
}

ToTemporalOverflow(options); // validate and ignore
let { month, day, referenceISOYear, calendar } = ParseTemporalMonthDayString(ToString(item));
let { month, day, referenceISOYear, calendar } = ParseTemporalMonthDayString(RequireString(item));
if (calendar === undefined) calendar = 'iso8601';
if (!IsBuiltinCalendar(calendar)) throw new RangeError(`invalid calendar identifier ${calendar}`);
calendar = ASCIILowercase(calendar);
Expand Down Expand Up @@ -1320,7 +1339,7 @@ export function ToTemporalTime(item, overflow = 'constrain') {
overflow
));
} else {
({ hour, minute, second, millisecond, microsecond, nanosecond } = ParseTemporalTimeString(ToString(item)));
({ hour, minute, second, millisecond, microsecond, nanosecond } = ParseTemporalTimeString(RequireString(item)));
RejectTime(hour, minute, second, millisecond, microsecond, nanosecond);
}
const TemporalPlainTime = GetIntrinsic('%Temporal.PlainTime%');
Expand All @@ -1337,7 +1356,7 @@ export function ToTemporalYearMonth(item, options) {
}

ToTemporalOverflow(options); // validate and ignore
let { year, month, referenceISODay, calendar } = ParseTemporalYearMonthString(ToString(item));
let { year, month, referenceISODay, calendar } = ParseTemporalYearMonthString(RequireString(item));
if (calendar === undefined) calendar = 'iso8601';
if (!IsBuiltinCalendar(calendar)) throw new RangeError(`invalid calendar identifier ${calendar}`);
calendar = ASCIILowercase(calendar);
Expand Down Expand Up @@ -1458,7 +1477,7 @@ export function ToTemporalZonedDateTime(item, options) {
} else {
let tzName, z;
({ year, month, day, hour, minute, second, millisecond, microsecond, nanosecond, tzName, offset, z, calendar } =
ParseTemporalZonedDateTimeString(ToString(item)));
ParseTemporalZonedDateTimeString(RequireString(item)));
timeZone = ToTemporalTimeZoneSlotValue(tzName);
if (z) {
offsetBehaviour = 'exact';
Expand Down Expand Up @@ -1984,7 +2003,7 @@ export function ToTemporalCalendarSlotValue(calendarLike) {
}
return calendarLike;
}
const identifier = ToString(calendarLike);
const identifier = RequireString(calendarLike);
if (IsBuiltinCalendar(identifier)) return ASCIILowercase(identifier);
let calendar;
try {
Expand Down Expand Up @@ -2103,7 +2122,7 @@ export function ToTemporalTimeZoneSlotValue(temporalTimeZoneLike) {
}
return temporalTimeZoneLike;
}
const identifier = ToString(temporalTimeZoneLike);
const identifier = RequireString(temporalTimeZoneLike);
const { tzName, offset, z } = ParseTemporalTimeZoneString(identifier);
if (tzName) {
// tzName is any valid identifier string in brackets, and could be an offset identifier
Expand Down
7 changes: 1 addition & 6 deletions polyfill/lib/timezone.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,7 @@ import {

export class TimeZone {
constructor(identifier) {
// Note: if the argument is not passed, GetCanonicalTimeZoneIdentifier(undefined) will throw.
// This check exists only to improve the error message.
if (arguments.length < 1) {
throw new RangeError('missing argument: identifier is required');
}
let stringIdentifier = ES.ToString(identifier);
let stringIdentifier = ES.RequireString(identifier);
const parseResult = ES.ParseTimeZoneIdentifier(identifier);
if (parseResult.offsetNanoseconds !== undefined) {
stringIdentifier = ES.FormatOffsetTimeZoneIdentifier(parseResult.offsetNanoseconds);
Expand Down
2 changes: 1 addition & 1 deletion polyfill/test262
Submodule test262 updated 321 files
39 changes: 23 additions & 16 deletions spec/abstractops.html
Original file line number Diff line number Diff line change
Expand Up @@ -567,16 +567,21 @@ <h1>
</emu-table>
</emu-clause>

<emu-clause id="sec-temporal-torelativetemporalobject" aoid="ToRelativeTemporalObject">
<h1>ToRelativeTemporalObject ( _options_ )</h1>
<p>
The abstract operation ToRelativeTemporalObject examines the value of the `relativeTo` property of its _options_ argument.
If this is not present, it returns *undefined*.
Otherwise, it attempts to return a Temporal.ZonedDateTime instance or Temporal.PlainDate instance, in order of preference, by converting the value.
If neither of those are possible, the operation throws a *RangeError*.
</p>
<emu-clause id="sec-temporal-torelativetemporalobject" type="abstract operation">
<h1>ToRelativeTemporalObject (
_options_: an Object
): either a normal completion containing either a Temporal.ZonedDateTime object or a Temporal.PlainDate object, or a throw completion</h1>
<dl class="header">
<dt>description</dt>
<dd>
It examines the value of the `relativeTo` property of its _options_ argument.
If the value is *undefined*, it returns *undefined*.
If the value is not a String or an Object, it throws a *TypeError*.
Otherwise, it attempts to return a Temporal.ZonedDateTime instance or Temporal.PlainDate instance, in order of preference, by converting the value.
If neither of those are possible, it throws a *RangeError*.
</dd>
</dl>
<emu-alg>
1. Assert: Type(_options_) is Object.
1. Let _value_ be ? Get(_options_, *"relativeTo"*).
1. If _value_ is *undefined*, return *undefined*.
1. Let _offsetBehaviour_ be ~option~.
Expand All @@ -601,8 +606,8 @@ <h1>ToRelativeTemporalObject ( _options_ )</h1>
1. If _offsetString_ is *undefined*, then
1. Set _offsetBehaviour_ to ~wall~.
1. Else,
1. Let _string_ be ? ToString(_value_).
1. Let _result_ be ? ParseTemporalRelativeToString(_string_).
1. If _value_ is not a String, throw a *TypeError* exception.
1. Let _result_ be ? ParseTemporalRelativeToString(_value_).
1. Let _offsetString_ be _result_.[[TimeZone]].[[OffsetString]].
1. Let _timeZoneName_ be _result_.[[TimeZone]].[[Name]].
1. If _timeZoneName_ is *undefined*, then
Expand Down Expand Up @@ -1869,8 +1874,10 @@ <h1>
1. Set _value_ to ? ToPositiveIntegerWithTruncation(_value_).
1. Set _value_ to 𝔽(_value_).
1. Else,
1. Assert: _Conversion_ is ~ToString~.
1. Set _value_ to ? ToString(_value_).
1. Assert: _Conversion_ is ~ToPrimitiveAndRequireString~.
1. NOTE: Non-primitive values are supported here for consistency with other fields, but such values must coerce to Strings.
1. Set _value_ to ? ToPrimitive(_value_, ~string~).
1. If _value_ is not a String, throw a *TypeError* exception.
1. Perform ! CreateDataPropertyOrThrow(_result_, _property_, _value_).
1. Else if _requiredFields_ is a List, then
1. If _requiredFields_ contains _property_, then
Expand Down Expand Up @@ -1908,7 +1915,7 @@ <h1>
</tr>
<tr>
<td>*"monthCode"*</td>
<td>~ToString~</td>
<td>~ToPrimitiveAndRequireString~</td>
<td>*undefined*</td>
</tr>
<tr>
Expand Down Expand Up @@ -1948,12 +1955,12 @@ <h1>
</tr>
<tr>
<td>*"offset"*</td>
<td>~ToString~</td>
<td>~ToPrimitiveAndRequireString~</td>
<td>*undefined*</td>
</tr>
<tr>
<td>*"era"*</td>
<td>~ToString~</td>
<td>~ToPrimitiveAndRequireString~</td>
<td>*undefined*</td>
</tr>
<tr>
Expand Down
6 changes: 3 additions & 3 deletions spec/calendar.html
Original file line number Diff line number Diff line change
Expand Up @@ -525,8 +525,8 @@ <h1>
1. Return _temporalCalendarLike_.[[Calendar]].
1. If ? ObjectImplementsTemporalCalendarProtocol(_temporalCalendarLike_) is *false*, throw a *TypeError* exception.
1. Return _temporalCalendarLike_.
1. Let _identifier_ be ? ToString(_temporalCalendarLike_).
1. Set _identifier_ to ? ParseTemporalCalendarString(_identifier_).
1. If _temporalCalendarLike_ is not a String, throw a *TypeError* exception.
1. Let _identifier_ be ? ParseTemporalCalendarString(_temporalCalendarLike_).
1. If IsBuiltinCalendar(_identifier_) is *false*, throw a *RangeError* exception.
1. Return the ASCII-lowercase of _identifier_.
</emu-alg>
Expand Down Expand Up @@ -1020,7 +1020,7 @@ <h1>Temporal.Calendar ( _id_ )</h1>
<emu-alg>
1. If NewTarget is *undefined*, then
1. Throw a *TypeError* exception.
1. Set _id_ to ? ToString(_id_).
1. If _id_ is not a String, throw a *TypeError* exception.
1. If IsBuiltinCalendar(_id_) is *false*, then
1. Throw a *RangeError* exception.
1. Return ? CreateTemporalCalendar(_id_, NewTarget).
Expand Down
4 changes: 2 additions & 2 deletions spec/duration.html
Original file line number Diff line number Diff line change
Expand Up @@ -912,8 +912,8 @@ <h1>
</dl>
<emu-alg>
1. If Type(_temporalDurationLike_) is not Object, then
1. Let _string_ be ? ToString(_temporalDurationLike_).
1. Return ? ParseTemporalDurationString(_string_).
1. If _temporalDurationLike_ is not a String, throw a *TypeError* exception.
1. Return ? ParseTemporalDurationString(_temporalDurationLike_).
1. If _temporalDurationLike_ has an [[InitializedTemporalDuration]] internal slot, then
1. Return ! CreateDurationRecord(_temporalDurationLike_.[[Years]], _temporalDurationLike_.[[Months]], _temporalDurationLike_.[[Weeks]], _temporalDurationLike_.[[Days]], _temporalDurationLike_.[[Hours]], _temporalDurationLike_.[[Minutes]], _temporalDurationLike_.[[Seconds]], _temporalDurationLike_.[[Milliseconds]], _temporalDurationLike_.[[Microseconds]], _temporalDurationLike_.[[Nanoseconds]]).
1. Let _result_ be a new Duration Record with each field set to 0.
Expand Down
6 changes: 4 additions & 2 deletions spec/instant.html
Original file line number Diff line number Diff line change
Expand Up @@ -518,8 +518,10 @@ <h1>ToTemporalInstant ( _item_ )</h1>
1. Return _item_.
1. If _item_ has an [[InitializedTemporalZonedDateTime]] internal slot, then
1. Return ! CreateTemporalInstant(_item_.[[Nanoseconds]]).
1. Let _string_ be ? ToString(_item_).
1. Let _epochNanoseconds_ be ? ParseTemporalInstant(_string_).
1. NOTE: This use of ToPrimitive allows Instant-like objects to be converted.
1. Set _item_ to ? ToPrimitive(_item_, ~string~).
1. If _item_ is not a String, throw a *TypeError* exception.
1. Let _epochNanoseconds_ be ? ParseTemporalInstant(_item_).
1. Return ! CreateTemporalInstant(_epochNanoseconds_).
</emu-alg>
</emu-clause>
Expand Down
4 changes: 2 additions & 2 deletions spec/plaindate.html
Original file line number Diff line number Diff line change
Expand Up @@ -756,8 +756,8 @@ <h1>ToTemporalDate ( _item_ [ , _options_ ] )</h1>
1. Let _fields_ be ? PrepareTemporalFields(_item_, _fieldNames_, «»).
1. Return ? CalendarDateFromFields(_calendar_, _fields_, _options_).
1. Perform ? ToTemporalOverflow(_options_).
1. Let _string_ be ? ToString(_item_).
1. Let _result_ be ? ParseTemporalDateString(_string_).
1. If _item_ is not a String, throw a *TypeError* exception.
1. Let _result_ be ? ParseTemporalDateString(_item_).
1. Assert: IsValidISODate(_result_.[[Year]], _result_.[[Month]], _result_.[[Day]]) is *true*.
1. Let _calendar_ be _result_.[[Calendar]].
1. If _calendar_ is *undefined*, set _calendar_ to *"iso8601"*.
Expand Down
4 changes: 2 additions & 2 deletions spec/plaindatetime.html
Original file line number Diff line number Diff line change
Expand Up @@ -916,8 +916,8 @@ <h1>ToTemporalDateTime ( _item_ [ , _options_ ] )</h1>
1. Let _result_ be ? InterpretTemporalDateTimeFields(_calendar_, _fields_, _options_).
1. Else,
1. Perform ? ToTemporalOverflow(_options_).
1. Let _string_ be ? ToString(_item_).
1. Let _result_ be ? ParseTemporalDateTimeString(_string_).
1. If _item_ is not a String, throw a *TypeError* exception.
1. Let _result_ be ? ParseTemporalDateTimeString(_item_).
1. Assert: IsValidISODate(_result_.[[Year]], _result_.[[Month]], _result_.[[Day]]) is *true*.
1. Assert: IsValidTime(_result_.[[Hour]], _result_.[[Minute]], _result_.[[Second]], _result_.[[Millisecond]], _result_.[[Microsecond]], _result_.[[Nanosecond]]) is *true*.
1. Let _calendar_ be _result_.[[Calendar]].
Expand Down
4 changes: 2 additions & 2 deletions spec/plainmonthday.html
Original file line number Diff line number Diff line change
Expand Up @@ -383,8 +383,8 @@ <h1>ToTemporalMonthDay ( _item_ [ , _options_ ] )</h1>
1. Perform ! CreateDataPropertyOrThrow(_fields_, *"year"*, 𝔽(_referenceISOYear_)).
1. Return ? CalendarMonthDayFromFields(_calendar_, _fields_, _options_).
1. Perform ? ToTemporalOverflow(_options_).
1. Let _string_ be ? ToString(_item_).
1. Let _result_ be ? ParseTemporalMonthDayString(_string_).
1. If _item_ is not a String, throw a *TypeError* exception.
1. Let _result_ be ? ParseTemporalMonthDayString(_item_).
1. Let _calendar_ be _result_.[[Calendar]].
1. If _calendar_ is *undefined*, set _calendar_ to *"iso8601"*.
1. If IsBuiltinCalendar(_calendar_) is *false*, throw a *RangeError* exception.
Expand Down
4 changes: 2 additions & 2 deletions spec/plaintime.html
Original file line number Diff line number Diff line change
Expand Up @@ -580,8 +580,8 @@ <h1>ToTemporalTime ( _item_ [ , _overflow_ ] )</h1>
1. Let _result_ be ? ToTemporalTimeRecord(_item_).
1. Set _result_ to ? RegulateTime(_result_.[[Hour]], _result_.[[Minute]], _result_.[[Second]], _result_.[[Millisecond]], _result_.[[Microsecond]], _result_.[[Nanosecond]], _overflow_).
1. Else,
1. Let _string_ be ? ToString(_item_).
1. Let _result_ be ? ParseTemporalTimeString(_string_).
1. If _item_ is not a String, throw a *TypeError* exception.
1. Let _result_ be ? ParseTemporalTimeString(_item_).
1. Assert: IsValidTime(_result_.[[Hour]], _result_.[[Minute]], _result_.[[Second]], _result_.[[Millisecond]], _result_.[[Microsecond]], _result_.[[Nanosecond]]) is *true*.
1. Return ! CreateTemporalTime(_result_.[[Hour]], _result_.[[Minute]], _result_.[[Second]], _result_.[[Millisecond]], _result_.[[Microsecond]], _result_.[[Nanosecond]]).
</emu-alg>
Expand Down
4 changes: 2 additions & 2 deletions spec/plainyearmonth.html
Original file line number Diff line number Diff line change
Expand Up @@ -497,8 +497,8 @@ <h1>ToTemporalYearMonth ( _item_ [ , _options_ ] )</h1>
1. Let _fields_ be ? PrepareTemporalFields(_item_, _fieldNames_, «»).
1. Return ? CalendarYearMonthFromFields(_calendar_, _fields_, _options_).
1. Perform ? ToTemporalOverflow(_options_).
1. Let _string_ be ? ToString(_item_).
1. Let _result_ be ? ParseTemporalYearMonthString(_string_).
1. If _item_ is not a String, throw a *TypeError* exception.
1. Let _result_ be ? ParseTemporalYearMonthString(_item_).
1. Let _calendar_ be _result_.[[Calendar]].
1. If _calendar_ is *undefined*, set _calendar_ to *"iso8601"*.
1. If IsBuiltinCalendar(_calendar_) is *false*, throw a *RangeError* exception.
Expand Down
Loading

0 comments on commit 7723794

Please sign in to comment.