Skip to content

fix(decode): preserve mapKeyConverter in Decoder#clone() - #295

Open
maximilliangrand wants to merge 1 commit into
msgpack:mainfrom
maximilliangrand:fix/decoder-clone-mapkeyconverter
Open

fix(decode): preserve mapKeyConverter in Decoder#clone()#295
maximilliangrand wants to merge 1 commit into
msgpack:mainfrom
maximilliangrand:fix/decoder-clone-mapkeyconverter

Conversation

@maximilliangrand

Copy link
Copy Markdown

Problem

Decoder#clone() is used for re-entrant decoding — for example an extension codec that decodes nested MessagePack on the same reusable Decoder instance (the pattern from #195). clone() copies every decoder option except mapKeyConverter, so any map decoded re-entrantly silently falls back to the default key converter instead of the one the caller configured.

Repro (converter upper-cases keys; extension type 1 re-enters the same decoder):

const decoder = new Decoder({ extensionCodec, mapKeyConverter: (k) => String(k).toUpperCase() });
// extension type 1 decode: (data) => new Wrapped(decoder.decode(data))
const out = decoder.decode(encoder.encode({ a: 1, nested: new Wrapped({ b: 2 }) }));
// top-level keys: ["A", "NESTED"]   (converter applied)
// out.NESTED.inner keys: ["b"]      (converter dropped — should be ["B"])

Fix

Copy mapKeyConverter in clone() alongside the other options (keyDecoder and the max* limits are already copied). One line, plus a regression test that decodes a nested map through an extension codec.

Full suite: 328 passing; lint and typecheck clean.

Decoder#clone() is used for re-entrant decoding (e.g. an extension codec
that decodes nested MessagePack on the same Decoder instance, issue msgpack#195).
It copied every decoder option except mapKeyConverter, so any map decoded
re-entrantly silently fell back to the default key converter instead of the
one the caller configured.

Copy mapKeyConverter in clone() alongside the other options, and add a
regression test that decodes a nested map through an extension codec.
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