Skip to content

Validate Timestamp values are within spec's range#3550

Open
oldergod wants to merge 1 commit intomasterfrom
bquenaudon.2026-03-18.timestamprange
Open

Validate Timestamp values are within spec's range#3550
oldergod wants to merge 1 commit intomasterfrom
bquenaudon.2026-03-18.timestamprange

Conversation

@oldergod
Copy link
Member

Fixes #3548

@oldergod oldergod force-pushed the bquenaudon.2026-03-18.timestamprange branch from c4c820e to 840d114 Compare March 18, 2026 12:41
@oldergod oldergod marked this pull request as ready for review March 18, 2026 12:41
@oldergod oldergod force-pushed the bquenaudon.2026-03-18.timestamprange branch from 840d114 to ee58484 Compare March 18, 2026 13:02
.setOrderedAt(
Timestamp.newBuilder()
.setSeconds(-631152000000L) // 1950-01-01T00:00:00.250Z.
.setSeconds(-631152000L) // 1950-01-01T00:00:00.250Z.
Copy link
Member Author

Choose a reason for hiding this comment

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

we were passing ms... instead of s !

Copy link
Member Author

Choose a reason for hiding this comment

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

interestingly, protoc didn't throw though?

Copy link
Collaborator

@JakeWharton JakeWharton left a comment

Choose a reason for hiding this comment

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

I have no real opinion on this. Is the adapter's encode functions the right places to throw, or should it be validated once in the constructor of the message?

@oldergod
Copy link
Member Author

Hmmm, I was matching protoc behavior but being able to instantiate an invalid object is definitely weird... I'll think a bit

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.

Timestamp ProtoAdapter silently encodes out-of-range Instant values

2 participants