s2s/keys: clarify minimum_valid_until_ts query #2191
Conversation
| A millisecond POSIX timestamp in milliseconds indicating when the returned | ||
| certificates will need to be valid until to be useful to the requesting server. | ||
| A millisecond POSIX timestamp indicating until when the returned | ||
| keys MUST at least be valid. |
There was a problem hiding this comment.
TBH, I personally find the existing wording easier to follow and the proposed wording sounds awkward to me. Aside from the redundancy regarding milliseconds, and calling it certificates instead of keys, I don't see any problem with the existing wording. But if you think it's unclear, maybe something like:
A millisecond POSIX timestamp. The returned keys must be valid at least until this time to be useful to the requesting server.
There was a problem hiding this comment.
That proposal is also more understandable, it would be nice to actually use RFC2119 keywords after they have been introduced at the beginning of the spec ^^
Not sure if the "to be useful to the requesting server" is adding anything useful though, it is kinda unnecessary and already implied in my opinion.
A millisecond POSIX timestamp. The returned keys MUST at least be valid until this timestamp.
There was a problem hiding this comment.
I believe the intention here is not to say that the returned key MUST be valid until this timestamp in the RFC2119 sense, but rather that the parameter expresses the minimum time that the requesting server wants the keys to be valid for.
That is, the intention here is to explain the meaning of the parameter, rather than to set a requirement for the response. The current wording doesn't set a requirement on the response, so it would be a spec change to make it into a requirement.
There was a problem hiding this comment.
The interpretation of the existing parameter could go either way here, exactly because defined language is not used (which is unfortunately all over in this spec).
Not having a requirement on the response for query parameters is... definitely unexpected reading a definition for said query parameters. Something along the lines of "The returned keys MAY be valid until at least this timestamp." would be more in line with this interpretation.
In my opinion, having such a parameter is kind of pointless if it does not impose any requirements (which, I agree, removing it would be a spec change. I don't see how requiring the parameter to have an effect would require a spec change though).
famfo
force-pushed
the
keys-query-valid
branch
from
43ff9d0 to
70f6749
Compare
Pull Request Checklist
Preview: https://pr2191--matrix-spec-previews.netlify.app