fix: browse() and list_datablocks() for V3 multi-frame EXPLORE (S7-1200 FW V4.5)

[tommasofaedo]

## Problem

On S7-1200 firmware V4.5 (V3 protocol), `list_datablocks()` and `browse()`
return empty results because the EXPLORE response for `0x8A11FFFF` spans
**multiple TPKT frames** and uses a different encoding than V1/V2 PLCs.

Three separate issues:

1. **Multi-frame response not collected** — the EXPLORE response is split into
   3 consecutive TPKT frames. The second and third frames are raw BLOB
   continuation data with no response header (only a V3 HMAC prefix). The
   existing code only reads the first frame.

2. **Wrong parser for V3 EXPLORE** — V3 PLCs return a zlib-compressed
   `PlcContentInfo` XML blob (magic `78 DA`), not the PObject tree that
   `_parse_explore_datablocks()` expects.

3. **`_parse_explore_fields` crashes on V3 attributes** — three bugs when
   parsing EXPLORE responses from V3 PLCs:
   - WSTRING dtype `0x15` not recognised (only `0x13` was checked)
   - Strings decoded as UTF-16-BE instead of UTF-8
   - BLOB skip logic misses an extra `0x00` byte that V3 PLCs insert before
     the VLQ length; WSTRING skip was also missing the data bytes

## Changes

### `s7/connection.py` — add `_collect_explore_frames()`

New method on `S7CommPlusConnection` that collects all continuation frames
after the first EXPLORE response frame.  Detection: a frame whose body
(after HMAC strip) is smaller than the reference size by more than 5 bytes
is the last fragment.

### `s7/_s7commplus_client.py`

- **`_parse_explore_datablocks_xml()`** — new parser that finds the `78 DA`
  zlib magic in the concatenated response, decompresses it, and extracts
  `Entity[@Id="Block" Header[@Type="DB"]]` nodes from the `PlcContentInfo`
  XML.  Falls back to `_parse_explore_datablocks()` if no zlib magic is
  found (backward compatible with V1/V2 PLCs).

- **`list_datablocks()`** — when `_session_key is not None` (V3 PLCs),
  builds the `0x8A11FFFF` EXPLORE payload, calls `_collect_explore_frames()`
  to gather all frames, then calls `_parse_explore_datablocks_xml()`.

- **`browse()`** — calls `_collect_explore_frames()` for each per-DB EXPLORE
  on V3 connections.

- **`_parse_explore_fields()`** — fixes for V3 PLCs:
  - Accept dtype `0x15` (WSTRING) in addition to `0x13` for name attributes
  - Decode name strings as UTF-8 (not UTF-16-BE)
  - BLOB skip: add 1 byte for the extra `0x00` before VLQ length
  - WSTRING skip: include `str_len` bytes after the VLQ

## Tested on

- **PLC:** Siemens S7-1200 CPU 1212C DC/DC/DC
- **Firmware:** V4.5
- **Protocol:** V3 (no TLS, no password)

`list_datablocks()` now correctly returns `[{"name": "Data_block_1",
"number": 100, "rid": 2316173412}]` where it previously returned `[]`.

## Known limitation (documented, not fixed)

On FW V4.5, DB field definitions and I/Q/M tag names are stored in zlib
BLOBs with a Siemens preset dictionary (magic `78 7D`, FDICT flag set, dict
checksum `58 14 B0 3B`).  Python's `zlib.decompress()` returns
`Z_NEED_DICT` — the preset dictionary is embedded in TIA Portal and has not
been published by Siemens.

As a result, `browse()` returns DB names and numbers but cannot enumerate
individual field names on V3 PLCs.  This is a protocol-level constraint,
not a code bug.

---

[gijzelaerr]

Review Summary

This PR adds V3 (S7-1200 FW V4.5) support for list_datablocks() and browse() — three distinct fixes for multi-frame collection, zlib-compressed XML parsing, and _parse_explore_fields V3 attribute encoding. Real-hardware tested. No malicious code.

Issues to address:

1. No unit tests. This is the biggest gap. The XML parser, the multi-frame collector, and the _parse_explore_fields fixes all have zero test coverage. At minimum: a test for _parse_explore_datablocks_xml with a synthetic zlib-compressed XML blob, and a test for the WSTRING/BLOB skip fixes.

2. XML entity expansion (XXE). ET.fromstring() uses the default parser which resolves external entities. Since the XML comes from a PLC (not user input), the risk is low, but for defense-in-depth consider defusedxml or at least ET.XMLParser(resolve_entities=False) — a malicious response could trigger entity expansion.

3. collect_explore_frames fragment detection is fragile. The "body shorter than reference by >5 bytes = last fragment" heuristic assumes all full-size frames are within 5 bytes of each other. If the PLC sends a legitimately short intermediate frame, it would be misdetected as the last. A more robust approach: use the V3 protocol's own termination signal (if available) or at least add a max-frame-count guard to prevent infinite loops.

4. collect_explore_frames has no size/count limits. A malformed V3 response could drive unbounded memory allocation. Add caps similar to _recv_reassembled_payload (16 MiB / 4096 fragments).

5. _build_explore_payload_v3 uses VLQ for ExploreId. The existing _build_explore_request was just fixed (in #749) to use fixed UInt32 for ExploreId because that's what real PLCs expect. Using VLQ here may work for V3 but diverges from the corrected convention — is this intentional?

6. BLOB skip offset += 1 for the extra 0x00 byte is V3-specific but runs unconditionally in _parse_explore_fields. If a V1/V2 response has a BLOB attribute, this would skip one byte too many. Guard it behind a V3 check.

7. Async parity. S7CommPlusAsyncClient.list_datablocks() and browse() are not updated — async callers on V3 PLCs still get empty results.

Positive notes:

• Uses xml.etree.ElementTree (safe, stdlib)
• V1/V2 fallback path preserved correctly
• The zlib magic detection (78 DA) is sound
• No legacy snap7/ files touched

Not ready to merge — needs tests, size caps, and the BLOB skip V3 guard.

[gijzelaerr]

ET.fromstring() uses the default XML parser which resolves external entities. While the XML comes from a PLC, for defense-in-depth consider at minimum disabling entity resolution. Python 3.8+ ET.fromstring is safe against XXE by default (entities are not expanded), so this is low-risk — but worth a comment noting the assumption.

[gijzelaerr]

No size or fragment-count cap. A malformed V3 response could loop indefinitely and allocate unbounded memory. Add limits similar to _recv_reassembled_payload (_MAX_REASSEMBLED_FRAGMENTS / _MAX_REASSEMBLED_BYTES).

Also, the "body shorter than reference by >5 bytes" heuristic is fragile — if the PLC sends a legitimately shorter intermediate frame, collection stops early and silently truncates the response.

[gijzelaerr]

The offset += 1 for the extra 0x00 byte before BLOB VLQ length is V3-specific, but this code runs for all protocol versions. If a V1/V2 EXPLORE response contains a BLOB attribute, this will skip one byte too many and corrupt all subsequent parsing. Guard with a V3 check, or pass the protocol version into this function.

[tommasofaedo]

Thank you for the detailed review! I've pushed two commits addressing all three points.

---

1. collect_explore_frames() — unbounded loop / fragile heuristic

Addressed in commit 906c877:

• Added fragment_count counter + cap check using the existing _MAX_REASSEMBLED_FRAGMENTS / _MAX_REASSEMBLED_BYTES constants (same limits as _recv_reassembled_payload). A malformed response now raises S7ConnectionError instead of looping indefinitely.
• Added if frag_len == 0: break as the primary termination criterion — matches the standard S7CommPlus end-of-stream trailer (0x72 ver 0x00 0x00) already used in _recv_reassembled_payload.
• Kept the "shorter-than-full-frame" heuristic as a fallback in case the trailer is absent (as observed on some V3 captures). Updated docstring to document both paths.

2. _parse_explore_fields() — V3-specific offset += 1 runs for all protocol versions

Addressed in commit d1fd133:

• Added protocol_version: int = 0 parameter (default 0 = V1/unknown, backward compatible).
• Guarded offset += 1 with if protocol_version >= ProtocolVersion.V3:.
• Updated browse() to pass self._connection._protocol_version.

3. ET.fromstring() XXE safety note

Addressed in commit d1fd133: added inline comment noting that ET.fromstring is safe against XXE by default in Python 3.8+ and that the XML source is the PLC (trusted local network device).

Let me know if anything else needs adjustment.

[gijzelaerr]

Thanks for the thorough follow-up — all three points look well-addressed:

1. Frame caps + frag_len == 0 primary termination — much better. Using the existing _MAX_REASSEMBLED_* constants and keeping the shorter-frame heuristic as a fallback (rather than primary) is the right call.

2. V3-guarded offset += 1 — passing protocol_version into _parse_explore_fields and guarding the extra byte cleanly fixes the V1/V2 regression risk.

3. XXE comment — good to have the explicit note.

Two remaining items from the original review that are still open:

• No unit tests — the PR adds ~170 lines of logic (XML parsing, multi-frame collection, V3 BLOB handling) with zero test coverage. Even a basic test for _parse_explore_datablocks_xml with a synthetic zlib-compressed XML blob would catch regressions.
• No async parity — async browse() / list_datablocks() don't get the V3 path. Not necessarily a blocker for V1 of this PR, but worth noting.

[gijzelaerr]

Thanks @tommasofaedo — I've pushed the remaining fixes (unit tests + async parity) to fix/browse-v3-multiframe-explore on top of your commits:

Unit tests (25 new tests in tests/test_s7_unit.py):

• TestBuildExplorePayloadV3: VLQ encoding, trailer format
• TestParseExploreDatablocksXml: zlib decompression, XML extraction, fallback, error paths
• TestParseExploreFieldsV3: UTF-8/UTF-16-BE auto-detect, WSTRING 0x15, BLOB skip V3 vs V1
• TestCollectExploreFrames: multi-frame concatenation, trailer termination, short-frame fallback, V3 HMAC strip, fragment caps

Async parity (s7/_s7commplus_async_client.py):

• _collect_explore_frames() async method
• V3 branch in list_datablocks() and browse()
• protocol_version passthrough to _parse_explore_fields

All 90 unit tests pass. Will merge from fix/browse-v3-multiframe-explore.

[tommasofaedo]

Thanks @gijzelaerr — the unit tests and async parity look great, happy to see this going in.

One heads-up on future validation that I want to be fully transparent about:

I no longer have access to the PLC this PR was validated on. The device this was tested against — an S7-1200 CPU 1212C, FW V4.5, plaintext V3 (no TLS, no password) — is no longer available to me. From now on the only S7-1200 hardware I can test against is a newer "G2" S7-1200, FW V4.1, and it differs in a few ways that are worth documenting explicitly so nobody is surprised if a follow-up regression shows up there:

Differences on the G2 unit vs. the G1 (FW V4.5) unit this PR was tested on:

1. TLS is mandatory. The G2 has "Only allow secure PG/PC and HMI communication" enforced, so a plaintext S7CommPlus session (as used by this PR's test setup) is rejected outright. The connection layer has to bring up TLS before any EXPLORE/browse traffic can flow.

2. TLS handshake is tunneled inside TPKT/COTP DT frames, not a plain ctx.wrap_socket() on the raw socket — the same scheme TIA Portal V21 uses. It is preceded by an empty COTP-DT activation frame (03 00 00 07 02 F0 00) sent right before the TLS ClientHello.

3. TLS 1.3, cipher TLS_AES_256_GCM_SHA384, and the Server Hello carries no CertificateRequest (no client cert needed).

4. The browse/EXPLORE logic in this PR is unchanged and still applies — the multi-frame 0x8A11FFFF collection and the 78 7D FDICT preset-dictionary blobs behave the same. The G2 uses the same preset-dictionary checksum (Adler-32 0xce9b821b) as the G1, so this PR's list_datablocks() path works identically once the TLS transport is up.

5. Symbolic-browse detail (not part of this PR): the I-area address now resolves correctly to %I0.0. The %I43.0 value comes from TIA's FDICT sample dictionary, not the real tag — browse_tags.py (feat: I/Q/M symbolic tag browse for S7-1200 FW V4.5 via reconstructed preset dictionary #756) now discards it and rebuilds the address from the authoritative ByteOffset attribute (%I{ByteOffset}.{bit}), so all three project tags resolve exactly. (This supersedes the earlier note about a residual I-area hardware-address quirk.)

Net effect for this PR: the changes here should be unaffected — the multi-frame EXPLORE parsing is transport-agnostic. But if a future issue on FW V4.1 needs reproducing, it will only be reproducible over TLS, since that's the only S7-1200 I can reach now. Just flagging so you have the full picture before merging and for anyone triaging V4.x reports later.

To be clear: I can still run further tests and keep developing/validating this and related work — just always on the S7-1200 G2 (FW V4.1, over TLS) from now on. So if you need anything reproduced or extended, I'm still able to help; it will just be against that hardware.

Thanks again for finishing the async/test side.

---

For the record — what I can still do on the G2 (FW V4.1, over TLS): to make the "I can still test" point concrete, here's what I've already confirmed working on the G2 after adapting the transport for TLS. Note that symbolic tag browsing itself is my browse_tags.py FDICT helper (the preset-dictionary work tracked separately in #756), not part of this PR — but it runs fine on the G2 over TLS:

Capability	G2 (FW V4.1, TLS)	Notes
TLS transport bring-up	✅	TLS 1.3 tunneled in TPKT/COTP DT frames, TLS_AES_256_GCM_SHA384
S7CommPlus session init	✅	after the empty COTP-DT activation frame
list_datablocks() — multi-frame EXPLORE (this PR)	✅	transport-agnostic, unchanged from G1
78 7D FDICT preset-dictionary blobs	✅	same Adler-32 0xce9b821b as G1
Symbolic I/Q/M tag browse (browse_tags.py, #756)	✅	3/3 project tags recovered correctly
Read tag values (I/Q/M)	✅	matches physical PLC state

Concretely, the three symbolic tags configured on the G2 all browse correctly over TLS:

Tag	Recovered	TIA truth	Result
input_1	%I0.0	%I0.0	✅ exact (rebuilt from ByteOffset, #756)
output_1	%Q0.1	%Q0.1	✅ exact
merker_1	%M0.2	%M0.2	✅ exact

So the effective capability set on the G2 is the same as on the G1 once TLS is up, and all three project tags now resolve to their exact TIA addresses.

---

Re: merge — happy to rebase. I notice the PR now shows a conflict against master (mergeable_state: dirty), so it can't be merged as-is. If you'd rather take it through this PR than from your local fix/browse-v3-multiframe-explore branch, just say the word and I'll rebase the branch onto current master and force-push to clear the conflict — happy to do it today so it's ready to go.