Skip to content

Commit

Permalink
Bug 1847182 - Part 5: Don't coerce non-string primitives to strings. …
Browse files Browse the repository at this point in the history 
…r=allstarschh

Implement the changes from <tc39/proposal-temporal#2574>.

Differential Revision: https://phabricator.services.mozilla.com/D185409
  • Loading branch information
@anba
anba committed Sep 13, 2023
1 parent ccd16a2 commit 61c17c9
Show file tree
Hide file tree
Showing 12 changed files with 104 additions and 78 deletions.
2 changes: 0 additions & 2 deletions js/public/friend/ErrorNumbers.msg
View file
Original file line number Diff line number Diff line change
Expand Up @@ -839,12 +839,10 @@ MSG_DEF(JSMSG_TEMPORAL_INVALID_PROPERTY, 1, JSEXN_RANGEERR, "invalid
MSG_DEF(JSMSG_TEMPORAL_INSTANT_INVALID, 0, JSEXN_RANGEERR, "epoch nanoseconds too large")
MSG_DEF(JSMSG_TEMPORAL_INSTANT_NONINTEGER, 1, JSEXN_RANGEERR, "Instant must be an integer, but received {0}")
MSG_DEF(JSMSG_TEMPORAL_INSTANT_BAD_DURATION, 1, JSEXN_RANGEERR, "duration \"{0}\" property must be zero")
MSG_DEF(JSMSG_TEMPORAL_INSTANT_PARSE_BAD_TYPE, 1, JSEXN_RANGEERR, "can't parse {0} as a Temporal.Instant")
MSG_DEF(JSMSG_TEMPORAL_TIMEZONE_INVALID_IDENTIFIER, 1, JSEXN_RANGEERR, "invalid time zone: {0}")
MSG_DEF(JSMSG_TEMPORAL_TIMEZONE_NANOS_RANGE, 1, JSEXN_RANGEERR, "nanoseconds out of range: {0}")
MSG_DEF(JSMSG_TEMPORAL_TIMEZONE_INCOMPATIBLE, 2, JSEXN_RANGEERR, "time zones \"{0}\" and \"{1}\" aren't compatible")
MSG_DEF(JSMSG_TEMPORAL_TIMEZONE_INSTANT_AMBIGUOUS, 0, JSEXN_RANGEERR, "instant is ambiguous")
MSG_DEF(JSMSG_TEMPORAL_TIMEZONE_PARSE_BAD_TYPE, 1, JSEXN_RANGEERR, "can't parse {0} as a Temporal.TimeZone")
MSG_DEF(JSMSG_TEMPORAL_DURATION_INVALID_SIGN, 2, JSEXN_RANGEERR, "duration value \"{0}\" has a mismatched sign: {1}")
MSG_DEF(JSMSG_TEMPORAL_DURATION_INVALID_NON_FINITE, 2, JSEXN_RANGEERR, "duration value \"{0}\" is {1}")
MSG_DEF(JSMSG_TEMPORAL_DURATION_MISSING_UNIT, 0, JSEXN_TYPEERR, "Duration-like objects must have at least one duration unit")
Expand Down
25 changes: 16 additions & 9 deletions js/src/builtin/temporal/Calendar.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -728,10 +728,12 @@ bool js::temporal::ToTemporalCalendar(JSContext* cx,
}

// Step 3.
Rooted<JSString*> str(cx, JS::ToString(cx, calendarLike));
if (!str) {
if (!calendarLike.isString()) {
ReportValueError(cx, JSMSG_UNEXPECTED_TYPE, JSDVG_IGNORE_STACK,
calendarLike, nullptr, "not a string");
return false;
}
Rooted<JSString*> str(cx, calendarLike.toString());

// Step 4.
Rooted<JSLinearString*> identifier(cx, ParseTemporalCalendarString(cx, str));
Expand Down Expand Up @@ -3697,24 +3699,29 @@ static bool CalendarConstructor(JSContext* cx, unsigned argc, Value* vp) {
}

// Step 2.
JSString* id = JS::ToString(cx, args.get(0));
if (!id) {
if (!args.requireAtLeast(cx, "Temporal.Calendar", 1)) {
return false;
}

Rooted<JSLinearString*> linear(cx, id->ensureLinear(cx));
if (!linear) {
if (!args[0].isString()) {
ReportValueError(cx, JSMSG_UNEXPECTED_TYPE, JSDVG_SEARCH_STACK, args[0],
nullptr, "not a string");
return false;
}

Rooted<JSLinearString*> identifier(cx, args[0].toString()->ensureLinear(cx));
if (!identifier) {
return false;
}

// Step 3.
linear = ThrowIfNotBuiltinCalendar(cx, linear);
if (!linear) {
identifier = ThrowIfNotBuiltinCalendar(cx, identifier);
if (!identifier) {
return false;
}

// Step 4.
auto* calendar = CreateTemporalCalendar(cx, args, linear);
auto* calendar = CreateTemporalCalendar(cx, args, identifier);
if (!calendar) {
return false;
}
Expand Down
12 changes: 8 additions & 4 deletions js/src/builtin/temporal/Duration.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -541,10 +541,12 @@ bool js::temporal::ToTemporalDurationRecord(JSContext* cx,
// Step 1.
if (!temporalDurationLike.isObject()) {
// Step 1.a.
Rooted<JSString*> string(cx, JS::ToString(cx, temporalDurationLike));
if (!string) {
if (!temporalDurationLike.isString()) {
ReportValueError(cx, JSMSG_UNEXPECTED_TYPE, JSDVG_IGNORE_STACK,
temporalDurationLike, nullptr, "not a string");
return false;
}
Rooted<JSString*> string(cx, temporalDurationLike.toString());

// Step 1.b.
return ParseTemporalDurationString(cx, string, result);
Expand Down Expand Up @@ -4056,10 +4058,12 @@ static bool ToRelativeTemporalObject(JSContext* cx, Handle<JSObject*> options,
}
} else {
// Step 6.a.
Rooted<JSString*> string(cx, JS::ToString(cx, value));
if (!string) {
if (!value.isString()) {
ReportValueError(cx, JSMSG_UNEXPECTED_TYPE, JSDVG_IGNORE_STACK, value,
nullptr, "not a string");
return false;
}
Rooted<JSString*> string(cx, value.toString());

// Step 6.b.
bool isUTC;
Expand Down
45 changes: 22 additions & 23 deletions js/src/builtin/temporal/Instant.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -684,6 +684,7 @@ static InstantObject* CreateTemporalInstant(JSContext* cx, const CallArgs& args,
Wrapped<InstantObject*> js::temporal::ToTemporalInstant(JSContext* cx,
Handle<Value> item) {
// Step 1.
Rooted<Value> primitiveValue(cx, item);
if (item.isObject()) {
JSObject* itemObj = &item.toObject();

Expand All @@ -697,22 +698,20 @@ Wrapped<InstantObject*> js::temporal::ToTemporalInstant(JSContext* cx,
auto epochInstant = ToInstant(zonedDateTime);
return CreateTemporalInstant(cx, epochInstant);
}
}

// Step 2.
Rooted<JSString*> string(cx, JS::ToString(cx, item));
if (!string) {
return nullptr;
// Steps 1.c-d.
if (!ToPrimitive(cx, JSTYPE_STRING, &primitiveValue)) {
return nullptr;
}
}

// The string representation of other types can never be parsed as an instant,
// so directly throw an error here. But still perform ToString first for
// possible side-effects.
if (!item.isString() && !item.isObject()) {
ReportValueError(cx, JSMSG_TEMPORAL_INSTANT_PARSE_BAD_TYPE,
JSDVG_IGNORE_STACK, item, nullptr);
// Step 2.
if (!primitiveValue.isString()) {
ReportValueError(cx, JSMSG_UNEXPECTED_TYPE, JSDVG_IGNORE_STACK,
primitiveValue, nullptr, "not a string");
return nullptr;
}
Rooted<JSString*> string(cx, primitiveValue.toString());

// Step 3.
Instant epochNanoseconds;
Expand All @@ -731,6 +730,7 @@ bool js::temporal::ToTemporalInstantEpochInstant(JSContext* cx,
Handle<Value> item,
Instant* result) {
// Step 1.
Rooted<Value> primitiveValue(cx, item);
if (item.isObject()) {
JSObject* itemObj = &item.toObject();

Expand All @@ -745,23 +745,22 @@ bool js::temporal::ToTemporalInstantEpochInstant(JSContext* cx,
*result = ToInstant(zonedDateTime);
return true;
}
}

// Step 2.
Rooted<JSString*> string(cx, JS::ToString(cx, item));
if (!string) {
return false;
// Steps 1.c-d.
if (!ToPrimitive(cx, JSTYPE_STRING, &primitiveValue)) {
return false;
}
}

// The string representation of other types can never be parsed as an instant,
// so directly throw an error here. The value is always on the stack, so
// JSDVG_SEARCH_STACK can be used for even better error reporting. But still
// perform ToString first for possible side-effects.
if (!item.isString() && !item.isObject()) {
ReportValueError(cx, JSMSG_TEMPORAL_INSTANT_PARSE_BAD_TYPE,
JSDVG_SEARCH_STACK, item, nullptr);
// Step 2.
if (!primitiveValue.isString()) {
// The value is always on the stack, so JSDVG_SEARCH_STACK can be used for
// better error reporting.
ReportValueError(cx, JSMSG_UNEXPECTED_TYPE, JSDVG_SEARCH_STACK,
primitiveValue, nullptr, "not a string");
return false;
}
Rooted<JSString*> string(cx, primitiveValue.toString());

// Steps 3-4.
Instant epochNanoseconds;
Expand Down
6 changes: 4 additions & 2 deletions js/src/builtin/temporal/PlainDate.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -470,10 +470,12 @@ static Wrapped<PlainDateObject*> ToTemporalDate(
}

// Step 5.
Rooted<JSString*> string(cx, JS::ToString(cx, item));
if (!string) {
if (!item.isString()) {
ReportValueError(cx, JSMSG_UNEXPECTED_TYPE, JSDVG_IGNORE_STACK, item,
nullptr, "not a string");
return nullptr;
}
Rooted<JSString*> string(cx, item.toString());

// Step 6.
PlainDate result;
Expand Down
6 changes: 4 additions & 2 deletions js/src/builtin/temporal/PlainDateTime.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -586,10 +586,12 @@ static Wrapped<PlainDateTimeObject*> ToTemporalDateTime(
}

// Step 4.b.
Rooted<JSString*> string(cx, JS::ToString(cx, item));
if (!string) {
if (!item.isString()) {
ReportValueError(cx, JSMSG_UNEXPECTED_TYPE, JSDVG_IGNORE_STACK, item,
nullptr, "not a string");
return nullptr;
}
Rooted<JSString*> string(cx, item.toString());

// Step 4.c.
Rooted<JSString*> calendarString(cx);
Expand Down
6 changes: 4 additions & 2 deletions js/src/builtin/temporal/PlainMonthDay.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -282,10 +282,12 @@ static Wrapped<PlainMonthDayObject*> ToTemporalMonthDay(
}

// Step 6.
Rooted<JSString*> string(cx, JS::ToString(cx, item));
if (!string) {
if (!item.isString()) {
ReportValueError(cx, JSMSG_UNEXPECTED_TYPE, JSDVG_IGNORE_STACK, item,
nullptr, "not a string");
return nullptr;
}
Rooted<JSString*> string(cx, item.toString());

// Step 7.
PlainDate result;
Expand Down
6 changes: 4 additions & 2 deletions js/src/builtin/temporal/PlainTime.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -647,10 +647,12 @@ static Wrapped<PlainTimeObject*> ToTemporalTime(JSContext* cx,
// Step 4.

// Step 4.a.
Rooted<JSString*> string(cx, JS::ToString(cx, item));
if (!string) {
if (!item.isString()) {
ReportValueError(cx, JSMSG_UNEXPECTED_TYPE, JSDVG_IGNORE_STACK, item,
nullptr, "not a string");
return nullptr;
}
Rooted<JSString*> string(cx, item.toString());

// Step 4.b.
if (!ParseTemporalTimeString(cx, string, &result)) {
Expand Down
6 changes: 4 additions & 2 deletions js/src/builtin/temporal/PlainYearMonth.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -249,10 +249,12 @@ static Wrapped<PlainYearMonthObject*> ToTemporalYearMonth(
}

// Step 5.
Rooted<JSString*> string(cx, JS::ToString(cx, item));
if (!string) {
if (!item.isString()) {
ReportValueError(cx, JSMSG_UNEXPECTED_TYPE, JSDVG_IGNORE_STACK, item,
nullptr, "not a string");
return nullptr;
}
Rooted<JSString*> string(cx, item.toString());

// Step 6.
PlainDate result;
Expand Down
22 changes: 18 additions & 4 deletions js/src/builtin/temporal/TemporalFields.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,20 @@ static mozilla::Maybe<TemporalField> ToTemporalField(JSContext* cx,
return mozilla::Nothing();
}

static JSString* ToPrimitiveAndRequireString(JSContext* cx,
Handle<Value> value) {
Rooted<Value> primitive(cx, value);
if (!ToPrimitive(cx, JSTYPE_STRING, &primitive)) {
return nullptr;
}
if (!primitive.isString()) {
ReportValueError(cx, JSMSG_UNEXPECTED_TYPE, JSDVG_IGNORE_STACK, primitive,
nullptr, "not a string");
return nullptr;
}
return primitive.toString();
}

static Value TemporalFieldDefaultValue(TemporalField field) {
switch (field) {
case TemporalField::Year:
Expand Down Expand Up @@ -230,7 +244,7 @@ static bool TemporalFieldConvertValue(JSContext* cx, TemporalField field,
case TemporalField::MonthCode:
case TemporalField::Offset:
case TemporalField::Era: {
JSString* str = JS::ToString(cx, value);
JSString* str = ToPrimitiveAndRequireString(cx, value);
if (!str) {
return false;
}
Expand Down Expand Up @@ -377,7 +391,7 @@ bool js::temporal::PrepareTemporalFields(
}
break;
case TemporalField::MonthCode: {
JSString* str = JS::ToString(cx, value);
JSString* str = ToPrimitiveAndRequireString(cx, value);
if (!str) {
return false;
}
Expand Down Expand Up @@ -423,15 +437,15 @@ bool js::temporal::PrepareTemporalFields(
}
break;
case TemporalField::Offset: {
JSString* str = JS::ToString(cx, value);
JSString* str = ToPrimitiveAndRequireString(cx, value);
if (!str) {
return false;
}
result.offset().set(str);
break;
}
case TemporalField::Era: {
JSString* str = JS::ToString(cx, value);
JSString* str = ToPrimitiveAndRequireString(cx, value);
if (!str) {
return false;
}
Expand Down
40 changes: 16 additions & 24 deletions js/src/builtin/temporal/TimeZone.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -849,25 +849,12 @@ bool js::temporal::ToTemporalTimeZone(JSContext* cx,
}

// Step 2.
Rooted<JSString*> identifier(cx, JS::ToString(cx, timeZoneLike));
if (!identifier) {
return false;
}

// The string representation of (most) other types can never be parsed as a
// time zone, so directly throw an error here. But still perform ToString
// first for possible side-effects.
//
// Numeric values are also accepted, because their ToString representation can
// be parsed as a time zone offset value, e.g. ToString(-10) == "-10", which
// is then interpreted as "-10:00".
//
// FIXME: spec issue - ToString for negative Numbers/BigInt also accepted?
if (!timeZoneLike.isString() && !timeZoneLike.isNumeric()) {
ReportValueError(cx, JSMSG_TEMPORAL_TIMEZONE_PARSE_BAD_TYPE,
JSDVG_IGNORE_STACK, timeZoneLike, nullptr);
if (!timeZoneLike.isString()) {
ReportValueError(cx, JSMSG_UNEXPECTED_TYPE, JSDVG_IGNORE_STACK,
timeZoneLike, nullptr, "not a string");
return false;
}
Rooted<JSString*> identifier(cx, timeZoneLike.toString());

// Steps 3-6.
return ToTemporalTimeZone(cx, identifier, result);
Expand Down Expand Up @@ -1755,22 +1742,27 @@ static bool TimeZoneConstructor(JSContext* cx, unsigned argc, Value* vp) {
}

// Step 2.
Rooted<JSString*> identifier(cx, JS::ToString(cx, args.get(0)));
if (!identifier) {
if (!args.requireAtLeast(cx, "Temporal.TimeZone", 1)) {
return false;
}

Rooted<JSLinearString*> linearIdentifier(cx, identifier->ensureLinear(cx));
if (!linearIdentifier) {
if (!args[0].isString()) {
ReportValueError(cx, JSMSG_UNEXPECTED_TYPE, JSDVG_SEARCH_STACK, args[0],
nullptr, "not a string");
return false;
}

Rooted<JSLinearString*> identifier(cx, args[0].toString()->ensureLinear(cx));
if (!identifier) {
return false;
}

Rooted<JSString*> canonical(cx);
Rooted<Value> offsetNanoseconds(cx);
if (IsTimeZoneOffsetStringPrefix(linearIdentifier)) {
if (IsTimeZoneOffsetStringPrefix(identifier)) {
// Step 3.
int64_t nanoseconds;
if (!ParseTimeZoneOffsetString(cx, linearIdentifier, &nanoseconds)) {
if (!ParseTimeZoneOffsetString(cx, identifier, &nanoseconds)) {
return false;
}
MOZ_ASSERT(std::abs(nanoseconds) < ToNanoseconds(TemporalUnit::Day));
Expand All @@ -1783,7 +1775,7 @@ static bool TimeZoneConstructor(JSContext* cx, unsigned argc, Value* vp) {
offsetNanoseconds.setNumber(nanoseconds);
} else {
// Step 4.
canonical = ValidateAndCanonicalizeTimeZoneName(cx, linearIdentifier);
canonical = ValidateAndCanonicalizeTimeZoneName(cx, identifier);
if (!canonical) {
return false;
}
Expand Down
6 changes: 4 additions & 2 deletions js/src/builtin/temporal/ZonedDateTime.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -345,10 +345,12 @@ static Wrapped<ZonedDateTimeObject*> ToTemporalZonedDateTime(
}
} else {
// Step 6.a.
Rooted<JSString*> string(cx, JS::ToString(cx, item));
if (!string) {
if (!item.isString()) {
ReportValueError(cx, JSMSG_UNEXPECTED_TYPE, JSDVG_IGNORE_STACK, item,
nullptr, "not a string");
return nullptr;
}
Rooted<JSString*> string(cx, item.toString());

// Case 1: 19700101Z[+02:00]
// { [[Z]]: true, [[OffsetString]]: undefined, [[Name]]: "+02:00" }
Expand Down

0 comments on commit 61c17c9

Please sign in to comment.