⚠ This page is served via a proxy. Original site: https://github.com
This service does not collect credentials or authentication data.
Skip to content

Conversation

@maxtropets
Copy link
Collaborator

@maxtropets maxtropets commented Jan 15, 2026

Yet another step towards #7542

Faced difficulties with error handling for optionals, as in

try {
  value = cbor->map_at(KEY)->as_TYPE();
}
catch (CBORError) {
  // ignore because optional ignores ALL errros
  // but we only want to ignore lookup, and still report wrong type or any other error
}

Doing two lookups as in

if (cbor->map_has(KEY)) {
  ...
}

needs reiterating, and simply

Value value;
try {
  value = cbor->map_at();
}
catch {
  // ignore
}

// convert to type with rethrow wrapper

doesn't work, because map_at() returns const T&.

At the end, I decided to categorise errors. Now each has a concrete Error included, which can be used to classify errors and either rethrow or silence.

❗ This changes the existing behaviour for TimestampedProtectedHeader with std::optional<uint64_t> msg_created_at;. Previously, it'd have silently avoid that field if it was negative, however it looks wrong to me, so we'll fail if the filed is present but ill-formed.

@maxtropets maxtropets force-pushed the f/cose-auth-wrappers branch from 36dbc07 to 347915e Compare January 15, 2026 21:40
@maxtropets maxtropets marked this pull request as ready for review January 15, 2026 21:50
@maxtropets maxtropets requested a review from a team as a code owner January 15, 2026 21:50
Copilot AI review requested due to automatic review settings January 15, 2026 21:50
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors COSE authentication parsing to use CBOR wrapper APIs instead of direct QCBOR calls. The refactoring introduces structured error categorization through an Error enum and updated CBORDecodeError class, enabling better error handling for optional header fields.

Changes:

  • Introduced categorized error handling with Error enum and enhanced CBORDecodeError class
  • Replaced QCBOR-based parsing with CBOR wrapper API calls in COSE authentication functions
  • Changed validation behavior for TimestampedProtectedHeader.msg_created_at to fail on negative values (previously silently ignored)

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
src/crypto/cbor.h Added Error enum and converted CBORDecodeError to a class with error code tracking
src/crypto/cbor.cpp Updated all error throwing sites to use new error codes and implemented CBORDecodeError class methods
src/endpoints/authentication/cose_auth.cpp Refactored COSE parsing to use CBOR wrappers with categorized error handling for optional fields

@maxtropets maxtropets self-assigned this Jan 15, 2026
@maxtropets maxtropets merged commit db100b5 into microsoft:main Jan 16, 2026
17 checks passed
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