From 02665ba32e5eff4ae80607c418c2647dd33b2d3f Mon Sep 17 00:00:00 2001 From: brian khuu Date: Wed, 31 Jul 2024 00:16:00 +1000 Subject: [PATCH 1/2] gguf_writer.py: add_array() should not add to kv store if empty --- gguf-py/gguf/gguf_writer.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/gguf-py/gguf/gguf_writer.py b/gguf-py/gguf/gguf_writer.py index ba6f53cda25a1..d9f04aeb77c7f 100644 --- a/gguf-py/gguf/gguf_writer.py +++ b/gguf-py/gguf/gguf_writer.py @@ -312,6 +312,8 @@ def add_string(self, key: str, val: str) -> None: self.add_key_value(key, val, GGUFValueType.STRING) def add_array(self, key: str, val: Sequence[Any]) -> None: + if not val: + return self.add_key_value(key, val, GGUFValueType.ARRAY) @staticmethod @@ -845,7 +847,14 @@ def _pack_val(self, val: Any, vtype: GGUFValueType, add_vtype: bool) -> bytes: encoded_val = val.encode("utf-8") if isinstance(val, str) else val kv_data += self._pack("Q", len(encoded_val)) kv_data += encoded_val - elif vtype == GGUFValueType.ARRAY and isinstance(val, Sequence) and val: + elif vtype == GGUFValueType.ARRAY: + + if not isinstance(val, Sequence): + raise ValueError("Invalid GGUF metadata array, expecting sequence") + + if not val: + raise ValueError("Invalid GGUF metadata array. Empty array") + if isinstance(val, bytes): ltype = GGUFValueType.UINT8 else: From e96d26340779119edd7b97e5cc8d0e57c8c68081 Mon Sep 17 00:00:00 2001 From: Brian Date: Wed, 31 Jul 2024 00:29:33 +1000 Subject: [PATCH 2/2] Apply suggestions from code review I was wondering if there was a specific reason for `if val` but good to hear we can safely use `len(val == 0` Co-authored-by: compilade --- gguf-py/gguf/gguf_writer.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gguf-py/gguf/gguf_writer.py b/gguf-py/gguf/gguf_writer.py index d9f04aeb77c7f..2e0b335eebed1 100644 --- a/gguf-py/gguf/gguf_writer.py +++ b/gguf-py/gguf/gguf_writer.py @@ -312,7 +312,7 @@ def add_string(self, key: str, val: str) -> None: self.add_key_value(key, val, GGUFValueType.STRING) def add_array(self, key: str, val: Sequence[Any]) -> None: - if not val: + if len(val) == 0: return self.add_key_value(key, val, GGUFValueType.ARRAY) @@ -852,7 +852,7 @@ def _pack_val(self, val: Any, vtype: GGUFValueType, add_vtype: bool) -> bytes: if not isinstance(val, Sequence): raise ValueError("Invalid GGUF metadata array, expecting sequence") - if not val: + if len(val) == 0: raise ValueError("Invalid GGUF metadata array. Empty array") if isinstance(val, bytes):