Skip to content

der: allow SetOf* duplicates#2272

Merged
tarcieri merged 1 commit intoRustCrypto:masterfrom
kraemv:fix/set_of_duplicates
Apr 3, 2026
Merged

der: allow SetOf* duplicates#2272
tarcieri merged 1 commit intoRustCrypto:masterfrom
kraemv:fix/set_of_duplicates

Conversation

@kraemv
Copy link
Copy Markdown
Contributor

@kraemv kraemv commented Apr 2, 2026

Hello everyone,

this PR addresses #2271 and disables checks for duplicate elements in SetOf* structs.
The main change is removing the check_duplicate function and changing the behavior for Ordering::Equal results.

I also changed the reject_duplicates tests to allow_duplicate tests that check if duplicates are valid.

I am happy to receive feedback on the changes!

@kraemv kraemv mentioned this pull request Apr 2, 2026

/// Ensure set elements are lexicographically ordered using [`DerOrd`].
fn check_der_ordering<T: DerOrd>(a: &T, b: &T) -> Result<(), Error> {
match a.der_cmp(b)? {
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 can probably just change to:

Suggested change
match a.der_cmp(b)? {
if a.der_cmp(b)? == Ordering::Greater {

@@ -531,8 +510,7 @@ fn der_sort<T: DerOrd>(slice: &mut [T]) -> Result<(), Error> {

while j > 0 {
match slice[j - 1].der_cmp(&slice[j])? {
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.

Likewise this can probably switch to an if statement

@tarcieri
Copy link
Copy Markdown
Member

tarcieri commented Apr 2, 2026

@kraemv would you also mind adding a #[deprecated] attribute to ErrorKind::SetDuplicate? Thanks!

@tarcieri
Copy link
Copy Markdown
Member

tarcieri commented Apr 3, 2026

I'll go ahead and fix up my suggestions after merging

@tarcieri tarcieri changed the title der: Allow SetOf* duplicates der: allow SetOf* duplicates Apr 3, 2026
@tarcieri tarcieri merged commit 223f522 into RustCrypto:master Apr 3, 2026
117 checks passed
tarcieri added a commit that referenced this pull request Apr 3, 2026
Followup to #2272 which addressed #2271 by allowing duplicates, as they
are allowed per X.680 and X.690.

Also includes some small logic cleanups.
tarcieri added a commit that referenced this pull request Apr 3, 2026
Followup to #2272 which addressed #2271 by allowing duplicates, as they
are allowed per X.680 and X.690.

Also includes some small logic cleanups.
tarcieri added a commit that referenced this pull request Apr 3, 2026
Followup to #2272 which addressed #2271 by allowing duplicates, as they
are allowed per X.680 and X.690.

Also includes some small logic cleanups.
@kraemv
Copy link
Copy Markdown
Contributor Author

kraemv commented Apr 4, 2026

Thank you for doing the remaining fixes @tarcieri !

@kraemv kraemv deleted the fix/set_of_duplicates branch April 4, 2026 17:04
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