search

hildjj / node-cbor

Encode and decode CBOR documents, with both easy mode, streaming mode, and SAX-style evented mode.
MIT License
356 stars 73 forks source link

Add option to not accept maps with duplicate keys #161

Closed KubqoA closed 2 years ago

KubqoA commented 2 years ago

This PR adds a Decoder option to prevent the decoder from parsing maps with duplicate keys, which is one of the valid ways to handle duplicate map keys as described in the CBOR RFC.

Citing from CBOR RFC 8949: 3.1 Major types, Major type 5

A map that has duplicate keys may be well-formed, but it is not valid, and thus it causes indeterminate decoding; see also Section 5.6.

5.6. Specifying Keys for Maps

When processing maps that exhibit entries with duplicate keys, a generic decoder might do one of the following:

  • Not accept maps with duplicate keys ...
  • Pass all map entries to the application ...
  • Lose some entries with duplicate keys, e.g., deliver only the final (or first) entry out of the entries with the same key ... Generic decoders need to document which of these three approaches they implement.

I believe this package chose the third option, although not documented, but in my use of CBOR I found it quite useful to have an option to not accept maps with duplicate keys. I decided to implement it via a Decoder option as to not break backwards compatibility.

hildjj commented 2 years ago

I agree this is interesting. I'll get to doing a full review in a day or two. In the meantime, can you add yourself to the contributors in the packages/cbor/package.json file, please?

https://github.com/hildjj/node-cbor/blob/815ba89b6e4f5431cf4db4dffb73431bf1b6cd8e/packages/cbor/package.json#L39

KubqoA commented 2 years ago

Thanks! Added myself to the contributors and also added a few more tests.

KubqoA commented 2 years ago

Hi, sorry to bother you, just wanted to ask if you had any time to look at the changes? Thanks :)

KubqoA commented 2 years ago

Made the changes, also found some eslint errors in my code, so fixed those as well. Hope everything is good now :), oh and thanks for your time to review this :+1: