Skip to content

Fix ReadMapStrIntf rejecting bin-typed map keys - #450

Merged
philhofer merged 2 commits into
tinylib:masterfrom
dealnews:fix-readmapstrintf-bin-keys
Aug 28, 2026
Merged

Fix ReadMapStrIntf rejecting bin-typed map keys#450
philhofer merged 2 commits into
tinylib:masterfrom
dealnews:fix-readmapstrintf-bin-keys

Conversation

@mleklund

Copy link
Copy Markdown
Contributor

ReadMapStrIntf reads map keys with a plain ReadString call, which only accepts the 'str' wire type. The rest of the library already accepts 'bin' as well - ReadMapKey and ReadMapKeyZC have supported it since 2014, and this function's own zero-copy sibling, ReadMapStrIntfBytes, already goes through ReadMapKeyZC.

Swaps the ReadString call for ReadMapKey, matching what ReadMapStrIntfBytes already does.

Fixes #449

ReadMapStrIntf reads map keys with a plain ReadString call, which only
accepts the 'str' wire type. The rest of the library already accepts
'bin' as well - ReadMapKey and ReadMapKeyZC have supported it since
2014, and this function's own zero-copy sibling, ReadMapStrIntfBytes,
already goes through ReadMapKeyZC.

Swaps the ReadString call for ReadMapKey, matching what
ReadMapStrIntfBytes already does.

Fixes tinylib#449

@klauspost klauspost left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

lgtm

Comment thread msgp/read.go Outdated
Hoist the key declaration out of the loop and pass key[:0] to
ReadMapKey so the previous key's storage gets reused. The buffer
grows to the longest key in the map and stops allocating after
that, which takes a 32-key decode from 128 to 97 allocs.

Adds a test for what buffer reuse can break: a nested map, and a
short key following a longer one, both read through a small reader
buffer so the refill path is exercised.
@philhofer
philhofer merged commit fd032cd into tinylib:master Aug 28, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ReadMapStrIntf does not accept 'bin' typed map keys, unlike the rest of the library

3 participants