Skip to content

Report an unhashable merged key as an error, not a panic - #19

Merged
reuvenharrison merged 1 commit into
v3from
fix/unhashable-merge-key
Oct 26, 2026
Merged

reuvenharrison merged 1 commit into
v3from
fix/unhashable-merge-key

Conversation

@reuvenharrison

Copy link
Copy Markdown
Collaborator

A mapping being merged has each of its keys looked up among the keys already seen, before the check that rejects a map or a sequence as a key. Hashing one of those panics, so this document took down the caller instead of returning an error:

<<:
  ? [1]
  : v
1: x

The sibling 1: x matters only because it makes the target a map[interface{}]interface{}, which is what a string-keyed decode would have rejected earlier.

The guard is a port of what go-yaml does upstream (setPossiblyUnhashableKey / getPossiblyUnhashableKey, present in go.yaml.in/yaml/v3 and absent from the v3.0.1 line this fork is based on), so the behaviour now matches: an ordinary yaml: error naming the unhashable type.

Found by fuzzing kin-openapi's loader, where the same document panicked inside LoadFromData. With this change the loader returns an error.

Tests: two cases added to unmarshalErrorTests, so both the Unmarshal and the Decoder paths cover them. Full suite passes, including -race.

🤖 Generated with Claude Code

https://claude.ai/code/session_017e285CvofYf6joqC98gTXz

A mapping being merged has each of its keys looked up among the keys
already seen, before the check that rejects a map or a sequence as a
key. Hashing one of those panics, so a document like

    <<:
      ? [1]
      : v
    1: x

took down the caller instead of returning an error. Guard the lookup and
the store the way go-yaml does upstream, which turns the panic into an
ordinary yaml error.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017e285CvofYf6joqC98gTXz
@reuvenharrison
reuvenharrison merged commit 787f992 into v3 Oct 26, 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.

1 participant