Validate Timestamp values are within spec's range#3550
Conversation
oldergod
force-pushed
the
bquenaudon.2026-03-18.timestamprange
branch
from
c4c820e to
840d114
Compare
oldergod
force-pushed
the
bquenaudon.2026-03-18.timestamprange
branch
from
840d114 to
ee58484
Compare
| .setOrderedAt( | ||
| Timestamp.newBuilder() | ||
| .setSeconds(-631152000000L) // 1950-01-01T00:00:00.250Z. | ||
| .setSeconds(-631152000L) // 1950-01-01T00:00:00.250Z. |
There was a problem hiding this comment.
we were passing ms... instead of s !
There was a problem hiding this comment.
interestingly, protoc didn't throw though?
JakeWharton
left a comment
JakeWharton
left a comment
There was a problem hiding this comment.
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?
|
Hmmm, I was matching protoc behavior but being able to instantiate an invalid object is definitely weird... I'll think a bit |
|
I looked for other range-checking code (in Wire), but couldn't find any. Makes me wonder whether lack of validation was a deliberate design decision? |
|
@mdub I don't think that this is deliberate, no. |
|
Not sure how we would gracefully validate that on creation since we're typealiasing to |
|
You'd validate in each constructor of a message containing the value |
|
I thought of it but say the request/response type of an RPC is timestamp? |
3 participants
Fixes #3548