Metadata API: Accept X.Y spec_version #1796
Conversation
jku
force-pushed
the
accept-two-part-spec-version
branch
from
4cd9e15 to
b857a37
Compare
Pull Request Test Coverage Report for Build 1751048969
💛 - Coveralls |
| if len(spec_list) not in [2, 3] or not all( | ||
| el.isdigit() for el in spec_list |
There was a problem hiding this comment.
this is a weird construct but black wants to lay it out this way -- I'll take suggestions to make it clearer though
There was a problem hiding this comment.
I don't have a better suggestion as well, but it's not so bad.
black splits all brackets so that we can see clearly what are the arguments which are the complicated part.
|
coverage claims none of the valueerrors are tested. I'll have to investigate that -- EDIT: done, bug fix commit added. |
| if spec_list[0] != SPECIFICATION_VERSION[0]: | ||
| raise ValueError(f"Unsupported spec_version {spec_version}") |
There was a problem hiding this comment.
So, finally, we don't want to log if there are differences in minor versions?
There was a problem hiding this comment.
could do a debug log I suppose... but realistically a large portion of metadata that is deserialized is likely to not match exactly (think updated clients loading older targets metadata that was created with lower spec_version).
| "no spec_version": '{"_type": "snapshot", "version": 1, "expires": "2030-01-01T00:00:00Z", "meta": {}}', | ||
| "no version": '{"_type": "snapshot", "spec_version": "1.0.0", "expires": "2030-01-01T00:00:00Z", "meta": {}}', | ||
| "no expires": '{"_type": "snapshot", "spec_version": "1.0.0", "version": 1, "meta": {}}', |
There was a problem hiding this comment.
Good catch. The data can even be valid, but because of the _type, it will fail...
There was a problem hiding this comment.
yeah -- one of those cases where parsing exception messages might have helped... but I'm still not willing to do that amount of work
All TUF implementations used to use "1.0" as the spec version and most of them have never modified that value since. Accept two-part spec_version for legacy compatibility: it is strictly speaking against the current spec (which requires semver) but there should be no harm in doing this and it allows us to deserialize metadata generated by e.g. go-tuf. Fixes theupdateframework#1751 Signed-off-by: Jussi Kukkonen <[email protected]>
Most of the test_invalid_signed_serialization subtests are currently failing because "_type": "signed" and then the test tries to deserialize them as Snapshot (which fails a type check). Correct the type to "snapshot" so that we can fail in the correct places during serialization instead. Signed-off-by: Jussi Kukkonen <[email protected]>
jku
force-pushed
the
accept-two-part-spec-version
branch
from
996f477 to
1695622
Compare
|
force pushed with one change:
|
All TUF implementations used to use "1.0" as the spec version and most
of them have never modified that value since.
Accept two-digit spec_version for legacy compatibility: it is strictly
speaking against the current spec (which requires semver) but there
should be no harm in doing this and it allows us to deserialize
metadata generated by e.g. go-tuf.
Fixes #1751
Signed-off-by: Jussi Kukkonen [email protected]
Please verify and check that the pull request fulfills the following
requirements: