Skip to content

fix: sort and de-duplicate reference numbers before grouping - #3500

Closed
prajakta128 wants to merge 1 commit into
OWASP:masterfrom
prajakta128:fix/group-number-ranges-sort
Closed

prajakta128 wants to merge 1 commit into
OWASP:masterfrom
prajakta128:fix/group-number-ranges-sort

Conversation

@prajakta128

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes an issue where CAPEC/ASVS/MASTG reference numbers could be rendered out of order or with duplicates when the source YAML mapping was not already sorted.

group_number_ranges() previously assumed that its input was sorted and de-duplicated before grouping consecutive numbers into ranges. Since the mapping files are hand-maintained, that assumption is not always valid.

This PR sorts and de-duplicates the numeric references before applying the existing range-grouping logic.

Changes

  • Sort reference numbers numerically before grouping.

  • Remove duplicate reference numbers.

  • Add regression tests for:

    • Unsorted mapping data.
    • Duplicate reference numbers.
  • Update the function docstring to document the input assumption and behavior.

Example

Before:

[20, 116, 117, 97, 112, 485]
→ 20, 116-117, 97, 112, 485

After:

[20, 116, 117, 97, 112, 485]
→ 20, 97, 112, 116-117, 485

Duplicate input such as:

[5, 5, 6]

now correctly renders as:

5-6

Testing

  • python -m pytest tests/scripts/convert_utest.py -v
  • 156 tests passed
  • 0 failures

Fixes #3499

group_number_ranges() assumed pre-sorted, de-duplicated input, but
source YAML mapping files are hand-maintained and aren't guaranteed
to be either. This caused CAPEC/ASVS/MASTG reference lists on
generated cards to render out of ascending order or with duplicates.

Fixes #<issue-number>

Signed-off-by: Prajakta <prajuu2812@gmail.com>
@prajakta128

Copy link
Copy Markdown
Contributor Author

Hi @sydseter @rewtd — noticed the ClusterFuzzLite PR fuzzing / PR (address) check is failing, wanted to flag it before review rather than leave it silent.

The crash is in capec_map_enricher_fuzzer, not in the code this PR touches (group_number_ranges() / check_make_list_into_text() in convert.py). The failure itself is:

Exception ignored in: <function _removeHandlerRef at ...>
TypeError: 'NoneType' object is not callable

That's happening inside Python's own logging module during interpreter shutdown, not in application code, and none of my changed functions are reachable from that fuzz target. The job log also shows ClusterFuzzLite couldn't download any previous build artifact to confirm whether this crash pre-dates this PR ("Could not download artifact... Build ... does not exist"), so it wasn't able to verify it as a regression from this change.

Is this a known/pre-existing flake on capec_map_enricher_fuzzer, or should I look into it separately? Happy to open a separate issue for it if it's new — just didn't want to bundle an unrelated fix into this PR.

@sydseter

Copy link
Copy Markdown
Collaborator

We don't always want the dash if there are number ranges that followes in a sequence. It's only the attribute that has the "_print" postfix where we want to use the dash. For all other attributes, they should not have it. This is because the identifiers are used in linking to external pages.

@sydseter

Copy link
Copy Markdown
Collaborator

Thank you for paying attention. Bug reported.

@sydseter

Copy link
Copy Markdown
Collaborator

Please be aware of the contributor guidelines: https://github.com/OWASP/cornucopia/blob/master/CONTRIBUTING.md
You should use our default pull-request template when creating pull-request.

I will close this pull-request, but feel free to open a new pull-request in the correct way.

@sydseter sydseter closed this Sep 17, 2026
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.

Card reference lists (CAPEC/ASVS/MASTG) render out of order and with duplicates when source YAML isn't pre-sorted

2 participants