WICG / webpackage

Web packaging format
Other
1.22k stars 114 forks source link

Sort order example for map keys in canonical CBOR may be incorrect #550

Closed casey closed 4 years ago

casey commented 4 years ago

I was just reading through the Signed HTTP Exchanges draft, and I think the given example for how keys in a CBOR map should be sorted is incorrect.

The example reads:

The keys in every map MUST be sorted in the bytewise
lexicographic order of their canonical encodings. For example,
the following keys are correctly sorted:

1. 10, encoded as 0A.
2. 100, encoded as 18 64.
3. -1, encoded as 20.
4. "z", encoded as 61 7A.
5. "aa", encoded as 62 61 61.
6. [100], encoded as 81 18 64.
7. [-1], encoded as 81 20.
8. false, encoded as F4.

However, section 3.9 of RFC 7049 states:

   o  The keys in every map must be sorted lowest value to highest.
      Sorting is performed on the bytes of the representation of the key
      data items without paying attention to the 3/5 bit splitting for
      major types.  (Note that this rule allows maps that have keys of
      different types, even though that is probably a bad practice that
      could lead to errors in some canonicalization implementations.)
      The sorting rules are:

      *  If two keys have different lengths, the shorter one sorts
         earlier;

      *  If two keys have the same length, the one with the lower value
         in (byte-wise) lexical order sorts earlier.

It's a little odd, but this reads to me as if keys should be sorted by key length first, and only then should keys with the same length be lexically sorted. This would give the sorting:

1. 10, encoded as 0A.
2. -1, encoded as 20.
3. false, encoded as F4.
4. 100, encoded as 18 64.
5. "z", encoded as 61 7A.
6. [-1], encoded as 81 20.
7. "aa", encoded as 62 61 61.
8. [100], encoded as 81 18 64.
jyasskin commented 4 years ago

This is something that's changing between RFC 7049 and CBORbis, where the new version defines two possible key orderings for individual protocols to pick between. We're using the newer one, that doesn't mix values of different major types. We're still compatible with RFC 7049, which says "Those protocols are free to define what they mean by a canonical format and what encoders and decoders are expected to do."

casey commented 4 years ago

Ah, gotcha, thank you for the clarification!

jimmywarting commented 1 year ago

This just bit me in the as. I tried using a cbor package that didn't sort the keys in my headers maps. Can this be loosen up a bit?

at the moment i get ERR_INVALID_WEB_BUNDLE if i do not sort them myself according to this logic.