Skip to content

Commit 560b52d

Browse files
committed
Only allow the ISO extended format in UtcOffset.parse
1 parent a63571f commit 560b52d

File tree

11 files changed

+121
-135
lines changed

11 files changed

+121
-135
lines changed

core/common/src/UtcOffset.kt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,8 @@ public expect class UtcOffset {
4242
*
4343
* Examples of valid strings:
4444
* - `Z` or `+00:00`, an offset of zero;
45-
* - `+05`, five hours;
46-
* - `-02`, minus two hours;
45+
* - `+05:00`, five hours;
46+
* - `-02:00`, minus two hours;
4747
* - `+03:30`, three hours and thirty minutes;
4848
* - `+01:23:45`, an hour, 23 minutes, and 45 seconds.
4949
*/

core/common/test/InstantTest.kt

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,16 @@ class InstantTest {
128128
"-04",
129129
)
130130

131-
val offsets = offsetStrings.map { UtcOffset.parse(it) }
131+
val offsetFormat = UtcOffset.Format {
132+
optional("Z") {
133+
offsetHours()
134+
optional {
135+
char(':'); offsetMinutesOfHour()
136+
optional { char(':'); offsetSecondsOfMinute() }
137+
}
138+
}
139+
}
140+
val offsets = offsetStrings.map { UtcOffset.parse(it, offsetFormat) }
132141

133142
for (instant in instants) {
134143
for (offsetIx in offsets.indices) {

core/common/test/TimeZoneTest.kt

Lines changed: 51 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -129,80 +129,80 @@ class TimeZoneTest {
129129
@Test
130130
fun newYorkOffset() {
131131
val test = TimeZone.of("America/New_York")
132-
val offset = UtcOffset.parse("-5")
132+
val offset = UtcOffset(hours = -5)
133133

134-
fun check(expectedOffset: String, dateTime: LocalDateTime) {
135-
assertEquals(UtcOffset.parse(expectedOffset), dateTime.toInstant(offset).offsetIn(test))
134+
fun check(expectedHours: Int, dateTime: LocalDateTime) {
135+
assertEquals(UtcOffset(hours = expectedHours), dateTime.toInstant(offset).offsetIn(test))
136136
}
137137

138-
check("-5", LocalDateTime(2008, 1, 1))
139-
check("-5", LocalDateTime(2008, 2, 1))
140-
check("-5", LocalDateTime(2008, 3, 1))
141-
check("-4", LocalDateTime(2008, 4, 1))
142-
check("-4", LocalDateTime(2008, 5, 1))
143-
check("-4", LocalDateTime(2008, 6, 1))
144-
check("-4", LocalDateTime(2008, 7, 1))
145-
check("-4", LocalDateTime(2008, 8, 1))
146-
check("-4", LocalDateTime(2008, 9, 1))
147-
check("-4", LocalDateTime(2008, 10, 1))
148-
check("-4", LocalDateTime(2008, 11, 1))
149-
check("-5", LocalDateTime(2008, 12, 1))
150-
check("-5", LocalDateTime(2008, 1, 28))
151-
check("-5", LocalDateTime(2008, 2, 28))
152-
check("-4", LocalDateTime(2008, 3, 28))
153-
check("-4", LocalDateTime(2008, 4, 28))
154-
check("-4", LocalDateTime(2008, 5, 28))
155-
check("-4", LocalDateTime(2008, 6, 28))
156-
check("-4", LocalDateTime(2008, 7, 28))
157-
check("-4", LocalDateTime(2008, 8, 28))
158-
check("-4", LocalDateTime(2008, 9, 28))
159-
check("-4", LocalDateTime(2008, 10, 28))
160-
check("-5", LocalDateTime(2008, 11, 28))
161-
check("-5", LocalDateTime(2008, 12, 28))
138+
check(-5, LocalDateTime(2008, 1, 1))
139+
check(-5, LocalDateTime(2008, 2, 1))
140+
check(-5, LocalDateTime(2008, 3, 1))
141+
check(-4, LocalDateTime(2008, 4, 1))
142+
check(-4, LocalDateTime(2008, 5, 1))
143+
check(-4, LocalDateTime(2008, 6, 1))
144+
check(-4, LocalDateTime(2008, 7, 1))
145+
check(-4, LocalDateTime(2008, 8, 1))
146+
check(-4, LocalDateTime(2008, 9, 1))
147+
check(-4, LocalDateTime(2008, 10, 1))
148+
check(-4, LocalDateTime(2008, 11, 1))
149+
check(-5, LocalDateTime(2008, 12, 1))
150+
check(-5, LocalDateTime(2008, 1, 28))
151+
check(-5, LocalDateTime(2008, 2, 28))
152+
check(-4, LocalDateTime(2008, 3, 28))
153+
check(-4, LocalDateTime(2008, 4, 28))
154+
check(-4, LocalDateTime(2008, 5, 28))
155+
check(-4, LocalDateTime(2008, 6, 28))
156+
check(-4, LocalDateTime(2008, 7, 28))
157+
check(-4, LocalDateTime(2008, 8, 28))
158+
check(-4, LocalDateTime(2008, 9, 28))
159+
check(-4, LocalDateTime(2008, 10, 28))
160+
check(-5, LocalDateTime(2008, 11, 28))
161+
check(-5, LocalDateTime(2008, 12, 28))
162162
}
163163

164164
// from 310bp
165165
@Test
166166
fun newYorkOffsetToDST() {
167167
val test = TimeZone.of("America/New_York")
168-
val offset = UtcOffset.parse("-5")
168+
val offset = UtcOffset(hours = -5)
169169

170-
fun check(expectedOffset: String, dateTime: LocalDateTime) {
171-
assertEquals(UtcOffset.parse(expectedOffset), dateTime.toInstant(offset).offsetIn(test))
170+
fun check(expectedHours: Int, dateTime: LocalDateTime) {
171+
assertEquals(UtcOffset(hours = expectedHours), dateTime.toInstant(offset).offsetIn(test))
172172
}
173173

174-
check("-5", LocalDateTime(2008, 3, 8))
175-
check("-5", LocalDateTime(2008, 3, 9))
176-
check("-4", LocalDateTime(2008, 3, 10))
177-
check("-4", LocalDateTime(2008, 3, 11))
178-
check("-4", LocalDateTime(2008, 3, 12))
179-
check("-4", LocalDateTime(2008, 3, 13))
180-
check("-4", LocalDateTime(2008, 3, 14))
174+
check(-5, LocalDateTime(2008, 3, 8))
175+
check(-5, LocalDateTime(2008, 3, 9))
176+
check(-4, LocalDateTime(2008, 3, 10))
177+
check(-4, LocalDateTime(2008, 3, 11))
178+
check(-4, LocalDateTime(2008, 3, 12))
179+
check(-4, LocalDateTime(2008, 3, 13))
180+
check(-4, LocalDateTime(2008, 3, 14))
181181
// cutover at 02:00 local
182-
check("-5", LocalDateTime(2008, 3, 9, 1, 59, 59, 999999999))
183-
check("-4", LocalDateTime(2008, 3, 9, 2, 0, 0, 0))
182+
check(-5, LocalDateTime(2008, 3, 9, 1, 59, 59, 999999999))
183+
check(-4, LocalDateTime(2008, 3, 9, 2, 0, 0, 0))
184184
}
185185

186186
// from 310bp
187187
@Test
188188
fun newYorkOffsetFromDST() {
189189
val test = TimeZone.of("America/New_York")
190-
val offset = UtcOffset.parse("-4")
190+
val offset = UtcOffset(hours = -4)
191191

192-
fun check(expectedOffset: String, dateTime: LocalDateTime) {
193-
assertEquals(UtcOffset.parse(expectedOffset), dateTime.toInstant(offset).offsetIn(test))
192+
fun check(expectedHours: Int, dateTime: LocalDateTime) {
193+
assertEquals(UtcOffset(hours = expectedHours), dateTime.toInstant(offset).offsetIn(test))
194194
}
195195

196-
check("-4", LocalDateTime(2008, 11, 1))
197-
check("-4", LocalDateTime(2008, 11, 2))
198-
check("-5", LocalDateTime(2008, 11, 3))
199-
check("-5", LocalDateTime(2008, 11, 4))
200-
check("-5", LocalDateTime(2008, 11, 5))
201-
check("-5", LocalDateTime(2008, 11, 6))
202-
check("-5", LocalDateTime(2008, 11, 7))
196+
check(-4, LocalDateTime(2008, 11, 1))
197+
check(-4, LocalDateTime(2008, 11, 2))
198+
check(-5, LocalDateTime(2008, 11, 3))
199+
check(-5, LocalDateTime(2008, 11, 4))
200+
check(-5, LocalDateTime(2008, 11, 5))
201+
check(-5, LocalDateTime(2008, 11, 6))
202+
check(-5, LocalDateTime(2008, 11, 7))
203203
// cutover at 02:00 local
204-
check("-4", LocalDateTime(2008, 11, 2, 1, 59, 59, 999999999))
205-
check("-5", LocalDateTime(2008, 11, 2, 2, 0, 0, 0))
204+
check(-4, LocalDateTime(2008, 11, 2, 1, 59, 59, 999999999))
205+
check(-5, LocalDateTime(2008, 11, 2, 2, 0, 0, 0))
206206
}
207207

208208
@Test

core/common/test/UtcOffsetTest.kt

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,11 @@ class UtcOffsetTest {
2626
"-01_00", "-01;00", "-01@00", "-01:AA",
2727
"@01:00")
2828

29+
val invalidStandaloneOffsets = listOf(
30+
"+0000", "+0100", "+1800", "+180000",
31+
"+4", "+0", "-0",
32+
)
33+
2934
val fixedOffsetTimeZoneIds = listOf(
3035
"UTC", "UTC+0", "GMT+01", "UT-01", "Etc/UTC"
3136
)
@@ -88,6 +93,9 @@ class UtcOffsetTest {
8893
for (v in invalidUtcOffsetStrings) {
8994
assertFailsWith<DateTimeFormatException>("Should fail: $v") { UtcOffset.parse(v) }
9095
}
96+
for (v in invalidStandaloneOffsets) {
97+
assertFailsWith<DateTimeFormatException>("Should fail: $v") { UtcOffset.parse(v) }
98+
}
9199
for (v in fixedOffsetTimeZoneIds) {
92100
assertFailsWith<DateTimeFormatException>("Time zone name should not be parsed as UtcOffset: $v") { UtcOffset.parse(v) }
93101
}
@@ -123,29 +131,21 @@ class UtcOffsetTest {
123131

124132

125133
check(offsetSeconds, "$sign${hours.pad()}:${minutes.pad()}:${seconds.pad()}", canonical = seconds != 0)
126-
check(offsetSeconds, "$sign${hours.pad()}${minutes.pad()}${seconds.pad()}")
127134
if (seconds == 0) {
128135
check(offsetSeconds, "$sign${hours.pad()}:${minutes.pad()}", canonical = offsetSeconds != 0)
129-
check(offsetSeconds, "$sign${hours.pad()}${minutes.pad()}")
130-
if (minutes == 0) {
131-
check(offsetSeconds, "$sign${hours.pad()}")
132-
check(offsetSeconds, "$sign$hours")
133-
}
134136
}
135137
}
136138
check(0, "+00:00")
137139
check(0, "-00:00")
138-
check(0, "+0")
139-
check(0, "-0")
140140
check(0, "Z", canonical = true)
141141
}
142142

143143
@Test
144144
fun equality() {
145145
val equalOffsets = listOf(
146-
listOf("Z", "+0", "+00", "+0000", "+00:00:00", "-00:00:00"),
147-
listOf("+4", "+04", "+04:00"),
148-
listOf("-18", "-1800", "-18:00:00"),
146+
listOf("Z", "+00:00", "-00:00", "+00:00:00", "-00:00:00"),
147+
listOf("+04:00", "+04:00:00"),
148+
listOf("-18:00", "-18:00:00"),
149149
)
150150
for (equalGroup in equalOffsets) {
151151
val offsets = equalGroup.map { UtcOffset.parse(it) }
@@ -167,4 +167,4 @@ class UtcOffsetTest {
167167
assertIs<FixedOffsetTimeZone>(timeZone)
168168
assertEquals(offset, timeZone.offset)
169169
}
170-
}
170+
}

core/commonJs/src/UtcOffset.kt

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@
66
package kotlinx.datetime
77

88
import kotlinx.datetime.internal.JSJoda.ZoneOffset as jtZoneOffset
9+
import kotlinx.datetime.internal.JSJoda.ChronoField as jtChronoField
10+
import kotlinx.datetime.internal.JSJoda.DateTimeFormatterBuilder as jtDateTimeFormatterBuilder
11+
import kotlinx.datetime.internal.JSJoda.ResolverStyle as jtResolverStyle
912
import kotlinx.datetime.format.*
1013
import kotlinx.datetime.serializers.UtcOffsetSerializer
1114
import kotlinx.serialization.Serializable
@@ -19,15 +22,17 @@ public actual class UtcOffset internal constructor(internal val zoneOffset: jtZo
1922
override fun toString(): String = zoneOffset.toString()
2023

2124
public actual companion object {
25+
private val format = jtDateTimeFormatterBuilder().appendOffsetId().toFormatter(jtResolverStyle.STRICT)
2226

2327
public actual val ZERO: UtcOffset = UtcOffset(jtZoneOffset.UTC)
2428

25-
public actual fun parse(offsetString: String): UtcOffset = try {
26-
jsTry { jtZoneOffset.of(offsetString) }.let(::UtcOffset)
29+
public actual fun parse(offsetString: String): UtcOffset = UtcOffset(seconds = try {
30+
jsTry { format.parse(offsetString).get(jtChronoField.OFFSET_SECONDS) }
2731
} catch (e: Throwable) {
32+
if (e.isJodaDateTimeParseException()) throw DateTimeFormatException(e)
2833
if (e.isJodaDateTimeException()) throw DateTimeFormatException(e)
2934
throw e
30-
}
35+
})
3136

3237
@Suppress("FunctionName")
3338
public actual fun Format(block: DateTimeFormatBuilder.WithUtcOffset.() -> Unit): DateTimeFormat<UtcOffset> =

core/jvm/src/UtcOffsetJvm.kt

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import kotlinx.datetime.serializers.UtcOffsetSerializer
1010
import kotlinx.serialization.Serializable
1111
import java.time.DateTimeException
1212
import java.time.ZoneOffset
13+
import java.time.format.DateTimeFormatterBuilder
1314

1415
@Serializable(with = UtcOffsetSerializer::class)
1516
public actual class UtcOffset(internal val zoneOffset: ZoneOffset) {
@@ -20,11 +21,12 @@ public actual class UtcOffset(internal val zoneOffset: ZoneOffset) {
2021
override fun toString(): String = zoneOffset.toString()
2122

2223
public actual companion object {
24+
private val format = DateTimeFormatterBuilder().appendOffsetId().toFormatter()
2325

2426
public actual val ZERO: UtcOffset = UtcOffset(ZoneOffset.UTC)
2527

2628
public actual fun parse(offsetString: String): UtcOffset = try {
27-
ZoneOffset.of(offsetString).let(::UtcOffset)
29+
format.parse(offsetString, ZoneOffset::from).let(::UtcOffset)
2830
} catch (e: DateTimeException) {
2931
throw DateTimeFormatException(e)
3032
}

core/native/src/TimeZone.kt

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
package kotlinx.datetime
1010

11+
import kotlinx.datetime.format.*
1112
import kotlinx.datetime.internal.*
1213
import kotlinx.datetime.serializers.*
1314
import kotlinx.serialization.Serializable
@@ -34,7 +35,7 @@ public actual open class TimeZone internal constructor() {
3435
}
3536
try {
3637
if (zoneId.startsWith("+") || zoneId.startsWith("-")) {
37-
return UtcOffset.parse(zoneId).asTimeZone()
38+
return UtcOffset.parse(zoneId, lenientOffsetFormat).asTimeZone()
3839
}
3940
if (zoneId == "UTC" || zoneId == "GMT" || zoneId == "UT") {
4041
return FixedOffsetTimeZone(UtcOffset.ZERO, zoneId)
@@ -43,14 +44,14 @@ public actual open class TimeZone internal constructor() {
4344
zoneId.startsWith("UTC-") || zoneId.startsWith("GMT-")
4445
) {
4546
val prefix = zoneId.take(3)
46-
val offset = UtcOffset.parse(zoneId.substring(3))
47+
val offset = UtcOffset.parse(zoneId.substring(3), lenientOffsetFormat)
4748
return when (offset.totalSeconds) {
4849
0 -> FixedOffsetTimeZone(offset, prefix)
4950
else -> FixedOffsetTimeZone(offset, "$prefix$offset")
5051
}
5152
}
5253
if (zoneId.startsWith("UT+") || zoneId.startsWith("UT-")) {
53-
val offset = UtcOffset.parse(zoneId.substring(2))
54+
val offset = UtcOffset.parse(zoneId.substring(2), lenientOffsetFormat)
5455
return when (offset.totalSeconds) {
5556
0 -> FixedOffsetTimeZone(offset, "UT")
5657
else -> FixedOffsetTimeZone(offset, "UT$offset")
@@ -159,3 +160,26 @@ public actual fun LocalDateTime.toInstant(offset: UtcOffset): Instant =
159160

160161
public actual fun LocalDate.atStartOfDayIn(timeZone: TimeZone): Instant =
161162
timeZone.atStartOfDay(this)
163+
164+
private val lenientOffsetFormat = UtcOffsetFormat.build {
165+
alternativeParsing(
166+
{
167+
offsetHours(Padding.NONE)
168+
},
169+
{
170+
isoOffset(
171+
zOnZero = false,
172+
useSeparator = false,
173+
outputMinute = WhenToOutput.IF_NONZERO,
174+
outputSecond = WhenToOutput.IF_NONZERO
175+
)
176+
}
177+
) {
178+
isoOffset(
179+
zOnZero = true,
180+
useSeparator = true,
181+
outputMinute = WhenToOutput.ALWAYS,
182+
outputSecond = WhenToOutput.IF_NONZERO
183+
)
184+
}
185+
}

core/native/src/UtcOffset.kt

Lines changed: 2 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -13,17 +13,16 @@ import kotlin.math.abs
1313

1414
@Serializable(with = UtcOffsetSerializer::class)
1515
public actual class UtcOffset private constructor(public actual val totalSeconds: Int) {
16-
private val id: String = zoneIdByOffset(totalSeconds)
1716

1817
override fun hashCode(): Int = totalSeconds
1918
override fun equals(other: Any?): Boolean = other is UtcOffset && this.totalSeconds == other.totalSeconds
20-
override fun toString(): String = id
19+
override fun toString(): String = format(Formats.ISO)
2120

2221
public actual companion object {
2322

2423
public actual val ZERO: UtcOffset = UtcOffset(totalSeconds = 0)
2524

26-
public actual fun parse(offsetString: String): UtcOffset = parse(offsetString, lenientFormat)
25+
public actual fun parse(offsetString: String): UtcOffset = parse(offsetString, Formats.ISO)
2726

2827
private fun validateTotal(totalSeconds: Int) {
2928
if (totalSeconds !in -18 * SECONDS_PER_HOUR..18 * SECONDS_PER_HOUR) {
@@ -112,26 +111,3 @@ public actual fun UtcOffset(hours: Int? = null, minutes: Int? = null, seconds: I
112111
UtcOffset.ofSeconds(seconds ?: 0)
113112
}
114113
}
115-
116-
private val lenientFormat = UtcOffsetFormat.build {
117-
alternativeParsing(
118-
{
119-
offsetHours(Padding.NONE)
120-
},
121-
{
122-
isoOffset(
123-
zOnZero = false,
124-
useSeparator = false,
125-
outputMinute = WhenToOutput.IF_NONZERO,
126-
outputSecond = WhenToOutput.IF_NONZERO
127-
)
128-
}
129-
) {
130-
isoOffset(
131-
zOnZero = true,
132-
useSeparator = true,
133-
outputMinute = WhenToOutput.ALWAYS,
134-
outputSecond = WhenToOutput.IF_NONZERO
135-
)
136-
}
137-
}

0 commit comments

Comments
 (0)