Skip to content

Implement java.io.Serializable for some of the classes #373

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 19 additions & 4 deletions core/api/kotlinx-datetime.api
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ public final class kotlinx/datetime/InstantKt {
public static final fun yearsUntil (Lkotlinx/datetime/Instant;Lkotlinx/datetime/Instant;Lkotlinx/datetime/TimeZone;)I
}

public final class kotlinx/datetime/LocalDate : java/lang/Comparable {
public final class kotlinx/datetime/LocalDate : java/io/Serializable, java/lang/Comparable {
public static final field Companion Lkotlinx/datetime/LocalDate$Companion;
public fun <init> (III)V
public fun <init> (ILjava/time/Month;I)V
Expand Down Expand Up @@ -342,7 +342,7 @@ public final class kotlinx/datetime/LocalDateKt {
public static final fun toLocalDate (Ljava/lang/String;)Lkotlinx/datetime/LocalDate;
}

public final class kotlinx/datetime/LocalDateTime : java/lang/Comparable {
public final class kotlinx/datetime/LocalDateTime : java/io/Serializable, java/lang/Comparable {
public static final field Companion Lkotlinx/datetime/LocalDateTime$Companion;
public fun <init> (IIIIIII)V
public synthetic fun <init> (IIIIIIIILkotlin/jvm/internal/DefaultConstructorMarker;)V
Expand Down Expand Up @@ -397,7 +397,7 @@ public final class kotlinx/datetime/LocalDateTimeKt {
public static final fun toLocalDateTime (Ljava/lang/String;)Lkotlinx/datetime/LocalDateTime;
}

public final class kotlinx/datetime/LocalTime : java/lang/Comparable {
public final class kotlinx/datetime/LocalTime : java/io/Serializable, java/lang/Comparable {
public static final field Companion Lkotlinx/datetime/LocalTime$Companion;
public fun <init> (IIII)V
public synthetic fun <init> (IIIIILkotlin/jvm/internal/DefaultConstructorMarker;)V
Expand Down Expand Up @@ -474,6 +474,21 @@ public final class kotlinx/datetime/MonthKt {
public static final fun getNumber (Lkotlinx/datetime/Month;)I
}

public final class kotlinx/datetime/Ser : java/io/Externalizable {
public static final field Companion Lkotlinx/datetime/Ser$Companion;
public static final field DATE_TAG I
public static final field DATE_TIME_TAG I
public static final field TIME_TAG I
public static final field UTC_OFFSET_TAG I
public fun <init> ()V
public fun <init> (ILjava/lang/Object;)V
public fun readExternal (Ljava/io/ObjectInput;)V
public fun writeExternal (Ljava/io/ObjectOutput;)V
}

public final class kotlinx/datetime/Ser$Companion {
}

public class kotlinx/datetime/TimeZone {
public static final field Companion Lkotlinx/datetime/TimeZone$Companion;
public fun equals (Ljava/lang/Object;)Z
Expand Down Expand Up @@ -501,7 +516,7 @@ public final class kotlinx/datetime/TimeZoneKt {
public static final fun toLocalDateTime (Lkotlinx/datetime/Instant;Lkotlinx/datetime/TimeZone;)Lkotlinx/datetime/LocalDateTime;
}

public final class kotlinx/datetime/UtcOffset {
public final class kotlinx/datetime/UtcOffset : java/io/Serializable {
public static final field Companion Lkotlinx/datetime/UtcOffset$Companion;
public fun <init> (Ljava/time/ZoneOffset;)V
public fun equals (Ljava/lang/Object;)Z
Expand Down
4 changes: 3 additions & 1 deletion core/jvm/src/Instant.kt
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ import java.time.Instant as jtInstant
import java.time.Clock as jtClock

@Serializable(with = InstantIso8601Serializer::class)
public actual class Instant internal constructor(internal val value: jtInstant) : Comparable<Instant> {
public actual class Instant internal constructor(
internal val value: jtInstant
) : Comparable<Instant> {

public actual val epochSeconds: Long
get() = value.epochSecond
Expand Down
6 changes: 5 additions & 1 deletion core/jvm/src/LocalDate.kt
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ import java.time.LocalDate as jtLocalDate
import kotlin.internal.*

@Serializable(with = LocalDateIso8601Serializer::class)
public actual class LocalDate internal constructor(internal val value: jtLocalDate) : Comparable<LocalDate> {
public actual class LocalDate internal constructor(
internal val value: jtLocalDate
) : Comparable<LocalDate>, java.io.Serializable {
public actual companion object {
public actual fun parse(input: CharSequence, format: DateTimeFormat<LocalDate>): LocalDate =
if (format === Formats.ISO) {
Expand Down Expand Up @@ -100,6 +102,8 @@ public actual class LocalDate internal constructor(internal val value: jtLocalDa
@PublishedApi
@JvmName("toEpochDays")
internal fun toEpochDaysJvm(): Int = value.toEpochDay().clampToInt()

private fun writeReplace(): Any = Ser(Ser.DATE_TAG, this)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The java.time classes also have a readObject method that always throws an exception with a comment that this is about defending against malicious streams. I'm not very familiar with Java's serialization, so I don't know if this is needed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found this post about serialization via writeReplace/readResolve. This section is about the always throwing readObject method. Without it, a hand crafted byte stream could cause deserialization of e.g. LocalDate without going through Ser.readResolve. This could violate invariants of the classes. I think it could for example sneak in null to the non-nullable property LocalDate.value.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Yes, deserialize("aced00057372001a6b6f746c696e782e6461746574696d652e4c6f63616c44617465618443f17dae33e70200014c00097472756556616c75657400154c6a6176612f74696d652f4c6f63616c446174653b787070" .hexToByteArray(HexFormat { bytes.byteSeparator = "" })) does indeed create a LocalDate whose value is null.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lukellmann, would you like to make a PR that fixes (and tests!) this problem?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure, but it will probably take a few days

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm on it but it will take a bit (I'm going through the spec).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @lukellmann! Do you have updates regarding this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've started implementing it, still refining the tests.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#522

Sorry that it took so long, I was initially trying to do more than what's in that PR (testing that classes stick to a certain pattern when implementing java.io.Serializable, see the other branches in my fork). But I've now shrunk it down to the most important fixes and might open additional PRs later.

}

/**
Expand Down
5 changes: 4 additions & 1 deletion core/jvm/src/LocalDateTimeJvm.kt
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ import java.time.format.DateTimeParseException
import java.time.LocalDateTime as jtLocalDateTime

@Serializable(with = LocalDateTimeIso8601Serializer::class)
public actual class LocalDateTime internal constructor(internal val value: jtLocalDateTime) : Comparable<LocalDateTime> {
public actual class LocalDateTime internal constructor(
internal val value: jtLocalDateTime
) : Comparable<LocalDateTime>, java.io.Serializable {

public actual constructor(year: Int, month: Int, day: Int, hour: Int, minute: Int, second: Int, nanosecond: Int) :
this(try {
Expand Down Expand Up @@ -110,6 +112,7 @@ public actual class LocalDateTime internal constructor(internal val value: jtLoc
public actual val ISO: DateTimeFormat<LocalDateTime> = ISO_DATETIME
}

private fun writeReplace(): Any = Ser(Ser.DATE_TIME_TAG, this)
}

/**
Expand Down
7 changes: 5 additions & 2 deletions core/jvm/src/LocalTimeJvm.kt
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,9 @@ import java.time.format.DateTimeParseException
import java.time.LocalTime as jtLocalTime

@Serializable(with = LocalTimeIso8601Serializer::class)
public actual class LocalTime internal constructor(internal val value: jtLocalTime) :
Comparable<LocalTime> {
public actual class LocalTime internal constructor(
internal val value: jtLocalTime
) : Comparable<LocalTime>, java.io.Serializable {

public actual constructor(hour: Int, minute: Int, second: Int, nanosecond: Int) :
this(
Expand Down Expand Up @@ -90,6 +91,8 @@ public actual class LocalTime internal constructor(internal val value: jtLocalTi
public actual val ISO: DateTimeFormat<LocalTime> get() = ISO_TIME

}

private fun writeReplace(): Any = Ser(Ser.TIME_TAG, this)
}

@Deprecated(
Expand Down
6 changes: 5 additions & 1 deletion core/jvm/src/UtcOffsetJvm.kt
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ import java.time.format.DateTimeFormatterBuilder
import java.time.format.*

@Serializable(with = UtcOffsetSerializer::class)
public actual class UtcOffset(internal val zoneOffset: ZoneOffset) {
public actual class UtcOffset(
internal val zoneOffset: ZoneOffset
): java.io.Serializable {
public actual val totalSeconds: Int get() = zoneOffset.totalSeconds

override fun hashCode(): Int = zoneOffset.hashCode()
Expand Down Expand Up @@ -44,6 +46,8 @@ public actual class UtcOffset(internal val zoneOffset: ZoneOffset) {
public actual val ISO_BASIC: DateTimeFormat<UtcOffset> get() = ISO_OFFSET_BASIC
public actual val FOUR_DIGITS: DateTimeFormat<UtcOffset> get() = FOUR_DIGIT_OFFSET
}

private fun writeReplace(): Any = Ser(Ser.UTC_OFFSET_TAG, this)
}

@Suppress("ACTUAL_FUNCTION_WITH_DEFAULT_ARGUMENTS")
Expand Down
67 changes: 67 additions & 0 deletions core/jvm/src/internal/Ser.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
/*
* Copyright 2019-2024 JetBrains s.r.o. and contributors.
* Use of this source code is governed by the Apache 2.0 License that can be found in the LICENSE.txt file.
*/

@file:Suppress("PackageDirectoryMismatch")
package kotlinx.datetime

import java.io.*

@PublishedApi // changing the class name would result in serialization incompatibility
internal class Ser(private var typeTag: Int, private var value: Any?) : Externalizable {
constructor() : this(0, null)

override fun writeExternal(out: ObjectOutput) {
out.writeByte(typeTag)
val value = this.value
when (typeTag) {
DATE_TAG -> {
value as LocalDate
out.writeLong(value.value.toEpochDay())
}
TIME_TAG -> {
value as LocalTime
out.writeLong(value.toNanosecondOfDay())
}
DATE_TIME_TAG -> {
value as LocalDateTime
out.writeLong(value.date.value.toEpochDay())
out.writeLong(value.time.toNanosecondOfDay())
}
UTC_OFFSET_TAG -> {
value as UtcOffset
out.writeInt(value.totalSeconds)
}
else -> throw IllegalStateException("Unknown type tag: $typeTag for value: $value")
}
}

override fun readExternal(`in`: ObjectInput) {
typeTag = `in`.readByte().toInt()
value = when (typeTag) {
DATE_TAG ->
LocalDate(java.time.LocalDate.ofEpochDay(`in`.readLong()))
TIME_TAG ->
LocalTime.fromNanosecondOfDay(`in`.readLong())
DATE_TIME_TAG ->
LocalDateTime(
LocalDate(java.time.LocalDate.ofEpochDay(`in`.readLong())),
LocalTime.fromNanosecondOfDay(`in`.readLong())
)
UTC_OFFSET_TAG ->
UtcOffset(seconds = `in`.readInt())
else -> throw IOException("Unknown type tag: $typeTag")
}
}

private fun readResolve(): Any = value!!

companion object {
private const val serialVersionUID: Long = 0L
const val DATE_TAG = 2
const val TIME_TAG = 3
const val DATE_TIME_TAG = 4
const val UTC_OFFSET_TAG = 10
}
}
87 changes: 87 additions & 0 deletions core/jvm/test/JvmSerializationTest.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
/*
* Copyright 2019-2024 JetBrains s.r.o. and contributors.
* Use of this source code is governed by the Apache 2.0 License that can be found in the LICENSE.txt file.
*/

package kotlinx.datetime

import java.io.*
import kotlin.test.*

class JvmSerializationTest {

@Test
fun serializeLocalTime() {
roundTripSerialization(LocalTime(12, 34, 56, 789))
roundTripSerialization(LocalTime.MIN)
roundTripSerialization(LocalTime.MAX)
expectedDeserialization(LocalTime(23, 59, 15, 995_003_220), "090300004e8a52680954")
}

@Test
fun serializeLocalDate() {
roundTripSerialization(LocalDate(2022, 1, 23))
roundTripSerialization(LocalDate.MIN)
roundTripSerialization(LocalDate.MAX)
expectedDeserialization(LocalDate(2024, 8, 12), "09020000000000004deb")
}

@Test
fun serializeLocalDateTime() {
roundTripSerialization(LocalDateTime(2022, 1, 23, 21, 35, 53, 125_123_612))
roundTripSerialization(LocalDateTime.MIN)
roundTripSerialization(LocalDateTime.MAX)
expectedDeserialization(LocalDateTime(2024, 8, 12, 10, 15, 0, 997_665_331), "11040000000000004deb0000218faedb9233")
}

@Test
fun serializeUtcOffset() {
roundTripSerialization(UtcOffset(hours = 3, minutes = 30, seconds = 15))
roundTripSerialization(UtcOffset(java.time.ZoneOffset.MIN))
roundTripSerialization(UtcOffset(java.time.ZoneOffset.MAX))
expectedDeserialization(UtcOffset.parse("-04:15:30"), "050affffc41e")
}

@Test
fun serializeTimeZone() {
assertFailsWith<NotSerializableException> {
roundTripSerialization(TimeZone.of("Europe/Moscow"))
}
}

private fun serialize(value: Any?): ByteArray {
val bos = ByteArrayOutputStream()
val oos = ObjectOutputStream(bos)
oos.writeObject(value)
return bos.toByteArray()
}

private fun deserialize(serialized: ByteArray): Any? {
val bis = ByteArrayInputStream(serialized)
ObjectInputStream(bis).use { ois ->
return ois.readObject()
}
}

private fun <T> roundTripSerialization(value: T) {
val serialized = serialize(value)
val deserialized = deserialize(serialized)
assertEquals(value, deserialized)
}

@OptIn(ExperimentalStdlibApi::class)
private fun expectedDeserialization(expected: Any, blockData: String) {
val serialized = "aced0005737200146b6f746c696e782e6461746574696d652e53657200000000000000000c0000787077${blockData}78"
val hexFormat = HexFormat { bytes.byteSeparator = "" }

try {
val deserialized = deserialize(serialized.hexToByteArray(hexFormat))
if (expected != deserialized) {
assertEquals(expected, deserialized, "Golden serial form: $serialized\nActual serial form: ${serialize(expected).toHexString(hexFormat)}")
}
} catch (e: Throwable) {
fail("Failed to deserialize $serialized\nActual serial form: ${serialize(expected).toHexString(hexFormat)}", e)
}
}

}