Skip to content

asn1 time fractional second support#33

Merged
JesusMcCloud merged 7 commits into
developmentfrom
feature/asn1Time
Jun 29, 2026
Merged

asn1 time fractional second support#33
JesusMcCloud merged 7 commits into
developmentfrom
feature/asn1Time

Conversation

@JesusMcCloud

Copy link
Copy Markdown
Contributor

No description provided.

@JesusMcCloud
JesusMcCloud requested a review from nodh June 23, 2026 07:43
@JesusMcCloud
JesusMcCloud marked this pull request as draft June 25, 2026 11:42
@JesusMcCloud
JesusMcCloud marked this pull request as ready for review June 25, 2026 14:52
@JesusMcCloud
JesusMcCloud force-pushed the feature/asn1Time branch 2 times, most recently from beec7b6 to 55ba446 Compare June 26, 2026 18:05

@nodh nodh left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks fine, only some suggestions

Comment thread CHANGELOG.md Outdated
Comment thread core/src/commonMain/kotlin/at/asitplus/awesn1/Asn1Time.kt

override val format: Format get() = Format.GENERALIZED

val fractionDigits: String get() = fractionalSeconds

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Who would use this property instead of accessing fractionalSeconds directly?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

good catch! makes no sense.

Format.GENERALIZED -> instant.encodeToAsn1GeneralizedTimePrimitive()
when (this) {
is Fractional -> {
val fraction = fractionDigits

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why not use fractionalSeconds directly?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

see above. i was sloppy

decodable = object : Asn1Decodable<Asn1Primitive, Asn1Time> {
@Throws(Asn1Exception::class)
override fun doDecode(src: Asn1Primitive): Asn1Time =
if (src.tag == Asn1Element.Tag.TIME_UTC) fromUtc(src.content) else decodeGeneralizedTimeToAsn1Time(src.content)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nitpick: This line is longer than 120 chars, so it might get reformatted in another PR leading to unrelated changes there

/** Constructs a whole-second [Asn1Time] from an [Instant]. Sub-second precision is dropped (see [SecondsCapped]). */
operator fun invoke(instant: Instant, formatOverride: Format? = null): Asn1Time {
return if (instant.nanosecondsOfSecond == 0) SecondsCapped(instant, formatOverride)
else if (formatOverride == Format.UTC) throw IllegalArgumentException("Cannot construct fractional UTC time")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nitpick: This line is longer than 120 chars, so it might get reformatted in another PR leading to unrelated changes there


/** RFC 5280 §4.1.2.5 cut-over: times in `[1950,2050)` use UTC TIME, everything else GENERALIZED TIME. */
private fun pickFormat(instant: Instant): Asn1Time.Format =
if (instant !in THRESHOLD_UTC_TIME..<THRESHOLD_GENERALIZED_TIME) Asn1Time.Format.GENERALIZED else Asn1Time.Format.UTC

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nitpick: This line is longer than 120 chars, so it might get reformatted in another PR leading to unrelated changes there

/**
* Returns a [Instant] with the same epoch seconds, but nanosecond precision capped
*/
fun Instant.secondsCapped() =Instant.fromEpochSeconds(this.epochSeconds) No newline at end of file

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nitpick: Missing space after =

}

/**
* Orders OIDs by their DER encoding ([bytes]) using unsigned lexicographic byte comparison — i.e. the

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This has nothing todo with the PR title

"T${s[8]}${s[9]}" + // hour
":${s[10]}${s[11]}" + // minute
":${s[12]}${s[13]}" // seconds
val sep = s.indexOf('.')

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nitpick: Would rename to sepIndex because it's not the separator . but its index.

@JesusMcCloud JesusMcCloud left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Gracias!

Comment thread core/src/commonMain/kotlin/at/asitplus/awesn1/Asn1Time.kt

override val format: Format get() = Format.GENERALIZED

val fractionDigits: String get() = fractionalSeconds

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

good catch! makes no sense.

Format.GENERALIZED -> instant.encodeToAsn1GeneralizedTimePrimitive()
when (this) {
is Fractional -> {
val fraction = fractionDigits

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

see above. i was sloppy

@JesusMcCloud
JesusMcCloud changed the base branch from feature/iterativeParsing to development June 29, 2026 14:33
@JesusMcCloud
JesusMcCloud merged commit 1f242f3 into development Jun 29, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants