Skip to content

fix: only allow minimally encoded CID tag#55

Open
vmx wants to merge 1 commit into
string-keys-onlyfrom
minimally-encoded-cid-tag
Open

fix: only allow minimally encoded CID tag#55
vmx wants to merge 1 commit into
string-keys-onlyfrom
minimally-encoded-cid-tag

Conversation

@vmx
Copy link
Copy Markdown
Member

@vmx vmx commented May 13, 2026

Integers should always be minimally encoded, so should be the header of a tag.

Integers should always be minimally encoded, so should be the header
of a tag.
@vmx vmx requested a review from rvagg May 13, 2026 23:00
Comment thread src/de.rs
name: "CBOR tag",
found: tag as u8,
}),
found: tag,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this is not helpful now when head != 0xd8 and you've pulled an extra byte when you didn't need to
maybe just have two if blocks here, pull head check & error, pull tag check and error

Comment thread tests/de.rs
DecodeError::Mismatch {
name: "CBOR tag",
found: 0xf7
found: 0xd9
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this passes whether we report head or the byte after it, since both are 0xd9. Some better inputs that distinguish:

  • over-wide encoding of tag 42, assertion should be found: 0xd9 (the head): 0xd9, 0x00, 0x2a, 0x66, 0x66, 0x6f, 0x6f, 0x62, 0x61, 0x72
  • minimally-encoded header but wrong tag number, assertion should be found: 0x29: 0xd8, 0x29

The first exercises the head-width check; the second exercises the tag-value check.

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.

2 participants