Refuse tags with no known size in strict parse_tag

CHANGES.md

@@ -14,6 +14,9 @@ Unreleased
 
 * Clock data with a milliseconds digit above 9 is now rejected, by the client when reading the clock and by the server
   when setting it. Previously `12 a3` decoded to `.130` (#966).
+* `parse_tag` now raises `ValueError` under `strict=True`, the default, for a type with no known size, such as
+  `DB1.DBB0:STRING` without a length. Previously this failed only when `.size` was read. `S5TIME` now has a size
+  and can be read, but not yet written (#974).
 * `read_szl` and `list_blocks_of_type` now check the PDU reference of follow-up replies, in both clients. Previously a
   stale fragment was added to the result (#967).
 

doc/API/tags.rst

@@ -102,7 +102,7 @@ Supported types
 ``UINT``, ``WORD``, ``DINT``, ``UDINT``, ``DWORD``, ``LINT``, ``ULINT``,
 ``LWORD``, ``REAL``, ``LREAL``, ``TIME``, ``LTIME``, ``TOD``, ``LTOD``,
 ``DATE``, ``DT``, ``LDT``, ``DTL``, ``STRING[n]``, ``WSTRING[n]``,
-``FSTRING[n]``.
+``FSTRING[n]``. ``S5TIME`` can be read but not yet written.
 
 Arrays are supported for any fixed-size type via ``[count]`` suffix.
 

snap7/tags.py

@@ -62,6 +62,7 @@
     "WORD": 2,
     "WCHAR": 2,
     "DATE": 2,
+    "S5TIME": 2,
     "DINT": 4,
     "UDINT": 4,
     "DWORD": 4,
@@ -440,30 +441,40 @@ def parse_tag(address: str, *, strict: bool = True, name: str = "") -> Tag:
     Args:
         address: Tag address string.
         strict: When ``True`` (default), require one of the dialect markers
-            above. Bare short forms like ``M7.1`` or ``IW22`` raise
+            above and a type with a known size. Bare short forms like ``M7.1``
+            or ``IW22``, unknown types and ``STRING`` without a length raise
             :class:`ValueError`. When ``False``, bare forms are dispatched
-            to the nodeS7 parser (which accepts them).
+            to the nodeS7 parser (which accepts them), and the type is not
+            checked until :attr:`Tag.size` is read.
         name: Optional tag name to store on the resulting Tag.
 
     Returns:
         A :class:`PLC4XTag` or :class:`NodeS7Tag` depending on the dialect
         detected.
 
     Raises:
-        ValueError: If the input is ambiguous under strict mode, or if
-            the selected parser fails to parse.
+        ValueError: If the input is ambiguous or its type has no known size
+            under strict mode, or if the selected parser fails to parse.
     """
     s = address.strip()
+    tag: Tag
     if "," in s:
-        return NodeS7Tag.parse(s, name)
-    if ":" in s:
-        return PLC4XTag.parse(s, name)
-    if strict:
+        tag = NodeS7Tag.parse(s, name)
+    elif ":" in s:
+        tag = PLC4XTag.parse(s, name)
+    elif strict:
         raise ValueError(
             f"Ambiguous tag syntax {address!r}: must contain ',' (nodeS7) "
             f"or ':TYPE' (PLC4X). Pass strict=False to accept bare short forms."
         )
-    return NodeS7Tag.parse(s, name)
+    else:
+        tag = NodeS7Tag.parse(s, name)
+    if strict:
+        try:
+            tag.size
+        except ValueError as e:
+            raise ValueError(f"Tag address {address!r} has a type with no known size: {tag.datatype}") from e
+    return tag
 
 
 # ---------------------------------------------------------------------------
@@ -527,6 +538,8 @@ def _decode_scalar(datatype: str, data: bytearray, bit: int) -> Any:
         return util.get_wchar(data, 0)
     if datatype == "DATE":
         return util.get_date(data, 0)
+    if datatype == "S5TIME":
+        return util.get_s5time(data, 0)
     if datatype == "DINT":
         return util.get_dint(data, 0)
     if datatype == "UDINT":

tests/test_tags.py

@@ -6,6 +6,7 @@
 import pytest
 
 from snap7.tags import NodeS7Tag, PLC4XTag, Tag, decode_tag, encode_tag, from_browse, load_csv, load_json, load_tia_xml, parse_tag
+from snap7 import util
 from snap7.type import Area
 
 
@@ -186,6 +187,31 @@ def test_private_names_still_work(self) -> None:
         assert _encode_tag is encode_tag
 
 
+class TestParseTagKnownSize:
+    @pytest.mark.parametrize("address", ["DB1.DBW0:BOGUS", "DB1.DBB0:STRING", "DB1:0:WSTRING", "MW0:BOGUS"])
+    def test_strict_refuses_a_type_without_a_size(self, address: str) -> None:
+        with pytest.raises(ValueError, match="no known size"):
+            parse_tag(address)
+
+    def test_not_strict_leaves_the_type_unchecked(self) -> None:
+        tag = parse_tag("DB1.DBW0:BOGUS", strict=False)
+        assert tag.datatype == "BOGUS"
+        with pytest.raises(ValueError, match="Unknown S7 type"):
+            tag.size
+
+    def test_explicit_parsers_are_unchanged(self) -> None:
+        assert PLC4XTag.parse("DB1.DBW0:BOGUS").datatype == "BOGUS"
+
+    def test_s5time_has_a_size_and_decodes(self) -> None:
+        tag = parse_tag("DB1.DBW0:S5TIME")
+        assert tag.size == 2
+        assert decode_tag(tag, bytearray(bytes.fromhex("0123"))) == util.get_s5time(bytearray(bytes.fromhex("0123")), 0)
+
+    def test_s5time_write_is_refused(self) -> None:
+        with pytest.raises(ValueError, match="S5TIME"):
+            encode_tag(parse_tag("DB1.DBW0:S5TIME"), bytearray(2), "10ms")
+
+
 class TestLoadCsv:
     """Load tags from CSV files and strings."""