Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MetaFile is missing from_bytes() or equivalent #1893

Closed
jku opened this issue Mar 2, 2022 · 2 comments · Fixed by #2273
Closed

MetaFile is missing from_bytes() or equivalent #1893

jku opened this issue Mar 2, 2022 · 2 comments · Fixed by #2273
Assignees
Labels
backlog Issues to address with priority for current development goals enhancement good first issue Bite-sized items for first time contributors

Comments

@jku
Copy link
Member

jku commented Mar 2, 2022

We already have TargetFile.from_data() / TargetFile.from_file() to calculate hashes and length for a target.
We should optionally support hashes and length for Metafile too (but likely the API is not same, see below).

This hasn't come up so far because we haven't really used the optional hashes in our metafiles... it did come up in theupdateframework/go-tuf#228.

Notes:

  • presumably most of the code currently in TargetFile.from_data()can be shared somehow
  • the situation is different in MetaFile and TargetFile for 2 reasons:
    • MetaFile needs version that comes from a Metadata -- obviously the implementation could parse the bytes it gets to construct Metadata... but that's not ideal because we can expect the Metadata is already in memory, reparsing is wasteful. The go-tuf generator code is a fine example of this.
    • A valid MetaFile only needs version: the API should not automatically lead to using hashes

potential API (maybe add from_file() too to be consistent):

class MetaFile(BaseFile):

    @classmethod
    def from_data(
        cls,
        version: int,
        data: Optional[Union[bytes, IO[bytes]]] = None,
        hash_algorithms: Optional[List[str]] = None,
    ) -> "MetaFile"
        # data is the Metadata bytes that the metafile represents.
        # Hashes and length should be included only if data is not None

This is consistent with TargetFile, but looks a bit odd: it's called from_data() but data argument can be None? Maybe data should not be optional: we should just document that Metafile.from_data(ver, data) constructor should only be used if hashes are wanted, by default MetaFile(ver) should be used.

@jku jku added the backlog Issues to address with priority for current development goals label Mar 9, 2022
@lukpueh
Copy link
Member

lukpueh commented Mar 16, 2022

For context:
The MetaFile class represents the METAFILES object from the TUF specification, which is used in timestamp metadata to reference the latest version of snapshot metadata, and in snapshot metadata to reference the latest version of targets metadata. The reference consists of the version and optionally length and/or hashes of the referenced metadata file(s).

@lukpueh lukpueh added the good first issue Bite-sized items for first time contributors label Mar 16, 2022
@Sush-Dew
Copy link

Sush-Dew commented Sep 9, 2022

@jku Could you assign this issue to me? I am a beginner and this issue seems a good first issue to resolve.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog Issues to address with priority for current development goals enhancement good first issue Bite-sized items for first time contributors
Projects
None yet
4 participants