geom: validate TWKB ID list count before narrowing to int

[peterstace]

Description

The TWKB parser has two pre-sized allocations, in parsePointArray and parseIDList, and both guard against an element count larger than the remaining input. #712 added those guards but wrote them differently. parsePointArray takes a uint64 and checks the count before narrowing it to an int; parseIDList took an int, so every caller cast the untrusted uvarint at the call site. The guard then needed a numIDs < 0 arm to catch a count that had already wrapped, and its error reported the wrapped value rather than the count given in the input:

number of IDs -1 exceeds remaining buffer size

This changes parseIDList to take a uint64 and bounds-check it before narrowing, and drops the int(...) cast at the five call sites. The multiplier is 1 rather than the dimension count, since each ID is a single signed varint of at least one byte. The two paths now report the same way:

number of points 18446744073709551615 exceeds remaining buffer size of 0 bytes
number of IDs 18446744073709551615 exceeds remaining buffer size of 0 bytes

#712 already stopped the panic on both paths. What this changes is the count reported in the ID-list error, and how the two guards are written.

Two smaller changes are included, both from review:

• nextGeometryCollection reported num polygons varint malformed while reading the collection's geometry count, and now reports num geometries. Every other collection parser names its own element type. Nothing asserts on the string.

• The CLAUDE.md guideline asking for a CHANGELOG entry "whenever making a change visible to users of this module" was stricter than the CHANGELOG's own history, which records internal work that is not externally detectable and records nothing about the wording of error messages. It now says what warrants an entry and names the cases too small to log.

Check List

Have you:

• Added unit tests? Yes. TestUnmarshalTWKBHugeCount gains a case for UnmarshalTWKBIDList, the second exported entry point into this guard, which had no coverage. The num geometries fix changes an error string that no test asserts on, so it adds no test.

• Add cmprefimpl tests? Not appropriate; cmprefimpl compares value semantics against PostGIS and GEOS, and this only changes error-message text on invalid input.

• Updated release notes? No. The Unreleased section of CHANGELOG.md already carries the entry from geom: fix panic in UnmarshalTWKB on huge element counts #712 describing count validation, and both message changes here apply to input that was already rejected. This is the case the CLAUDE.md change above now covers explicitly.

• Updated the README.md? Not applicable; no new functionality.

Related Issue

• Follow-up to geom: fix panic in UnmarshalTWKB on huge element counts #712, which added both guards. No separate issue.

Benchmark Results

• Not applicable; the change only affects the malformed-input rejection path.

[Copilot]

Pull request overview

This PR aligns TWKB ID-list count validation with the existing point-array guard by accepting the untrusted element count as a uint64, validating it against remaining input bytes before narrowing to int, and improving the resulting error reporting on malformed inputs.

Changes:

• Change parseIDList to accept uint64, validate before int narrowing, and improve the “remaining buffer size” error message.
• Remove int(...) casts at the parseIDList call sites across TWKB collection parsing paths.
• Extend TestUnmarshalTWKBHugeCount to cover the UnmarshalTWKBIDList entry point.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
geom/twkb_parser.go	Updates ID-list count validation and call sites to avoid overflow-before-check and improve error reporting.
geom/twkb_test.go	Adds coverage for the UnmarshalTWKBIDList entry point on huge element counts.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.