Skip to content

Small bugfixes, defensive checks, and improvements#753

Merged
fxamacker merged 4 commits intomasterfrom
fxamacker/small-bugfixes
Mar 28, 2026
Merged

Small bugfixes, defensive checks, and improvements#753
fxamacker merged 4 commits intomasterfrom
fxamacker/small-bugfixes

Conversation

@fxamacker
Copy link
Copy Markdown
Owner

@fxamacker fxamacker commented Mar 28, 2026

This PR fixes 4 minor issues found during refactoring:

NOTE: It is generally good practice to avoid using floating point to store epoch time for obvious reasons (even when not using CBOR).

This issue doesn't affect CBOR encoding or decoding.

Previously, the diagnostic functions which are used
to translate CBOR into Extended Diagnostic Notation
(human readable text for debugging) were rejecting
valid U+FFFD character.

This commit fixes Diagnose() and DiagnoseFirst() to
no longer reject valid U+FFFD character.
This refactor does not change actual behavior of the CBOR codec,
but it improves conformance to Go's sort.Interface.

This commit does not change behavior because:
- sort is used to deduplicate struct fields and fields with
  identical index are deduplicated
- sort is used to order struct fields after deduplication and
  we don't expect identical fields after deduplication
- sort is used to order map keys for encoding and we don't expect
  identical Go map keys
This commit adds a bounds check when decoding floating-point epoch time,
because extreme values (hundreds of billions of years) can exceed
the range of int64(seconds) if it was encoded using a different library.
This commit always returns a copy of cborNil slice when RawMessage
is empty.

Previously, RawMessage.MarshalCBOR() returned an internal cborNil
slice when RawMessage is empty.  This did not prevent the possibility
of external callers accidentally mutating the internal cborNil slice.

This issue only affects external direct call of RawMessage.MarshalCBOR.

The Marhsal() and Encode() functions already return a copy of data, so
they were not affected by this issue.
@fxamacker fxamacker self-assigned this Mar 28, 2026
@fxamacker fxamacker added bugfix improvement improvement that does not add new feature labels Mar 28, 2026
Copy link
Copy Markdown
Contributor

@x448 x448 left a comment

Choose a reason for hiding this comment

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

Amazing work!

@fxamacker fxamacker changed the base branch from fxamacker/refactor-and-add-tests to master March 28, 2026 23:55
@fxamacker fxamacker merged commit 59d62f5 into master Mar 28, 2026
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix improvement improvement that does not add new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants