Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -569,7 +569,7 @@ def test_metadata_targets(self):
hashes = {
"sha256": "141f740f53781d1ca54b8a50af22cbf74e44c21a998fa2a8a05aaac2c002886b",
"sha512": "ef5beafa16041bcdd2937140afebd485296cd54f7348ecd5a4d035c09759608de467a7ac0eb58753d0242df873c305e8bffad2454aa48f44480f15efae1cacd0"
},
}

fileinfo = TargetFile(length=28, hashes=hashes)

Expand Down
40 changes: 26 additions & 14 deletions tuf/api/metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -702,6 +702,19 @@ def _verify_length(
f"expected length {expected_length}"
)

@staticmethod
def _validate_hashes(hashes: Dict[str, str]) -> None:
if not hashes:
raise ValueError("Hashes must be a non empty dictionary")
for key, value in hashes.items():
if not (isinstance(key, str) and isinstance(value, str)):
raise TypeError("Hashes items must be strings")

@staticmethod
def _validate_length(length: int) -> None:
if length <= 0:
raise ValueError(f"Length must be > 0, got {length}")


class MetaFile(BaseFile):
"""A container with information about a particular metadata file.
Expand Down Expand Up @@ -730,6 +743,14 @@ def __init__(
hashes: Optional[Dict[str, str]] = None,
unrecognized_fields: Optional[Mapping[str, Any]] = None,
) -> None:

if version <= 0:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: these days I had a thought that it's probably easier to read if we use if version < 1 when doing this check.
What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I find <= 0 very readable

raise ValueError(f"Metafile version must be > 0, got {version}")
if length is not None:
self._validate_length(length)
if hashes is not None:
self._validate_hashes(hashes)

self.version = version
self.length = length
self.hashes = hashes
Expand All @@ -742,12 +763,6 @@ def from_dict(cls, meta_dict: Dict[str, Any]) -> "MetaFile":
length = meta_dict.pop("length", None)
hashes = meta_dict.pop("hashes", None)

# Do some basic input validation
if version <= 0:
raise ValueError(f"Metafile version must be > 0, got {version}")
if length is not None and length <= 0:
raise ValueError(f"Metafile length must be > 0, got {length}")

# All fields left in the meta_dict are unrecognized.
return cls(version, length, hashes, meta_dict)

Expand Down Expand Up @@ -778,8 +793,7 @@ def verify_length_and_hashes(self, data: Union[bytes, BinaryIO]):
if self.length is not None:
self._verify_length(data, self.length)

# Skip the check in case of an empty dictionary too
if self.hashes:
if self.hashes is not None:
self._verify_hashes(data, self.hashes)


Expand Down Expand Up @@ -1033,6 +1047,10 @@ def __init__(
hashes: Dict[str, str],
unrecognized_fields: Optional[Mapping[str, Any]] = None,
) -> None:

self._validate_length(length)
self._validate_hashes(hashes)

self.length = length
self.hashes = hashes
self.unrecognized_fields = unrecognized_fields or {}
Expand All @@ -1049,12 +1067,6 @@ def from_dict(cls, target_dict: Dict[str, Any]) -> "TargetFile":
length = target_dict.pop("length")
hashes = target_dict.pop("hashes")

# Do some basic validation checks
if length <= 0:
raise ValueError(f"Targetfile length must be > 0, got {length}")
if not hashes:
raise ValueError("Missing targetfile hashes")

# All fields left in the target_dict are unrecognized.
return cls(length, hashes, target_dict)

Expand Down