Skip to content

Enable validator for test 6.3.11 in CSAF 2.1#690

Closed
georgbramm wants to merge 1 commit into
csaf-rs:mainfrom
Fraunhofer-AISEC:fix-6.3.11-2.1
Closed

Enable validator for test 6.3.11 in CSAF 2.1#690
georgbramm wants to merge 1 commit into
csaf-rs:mainfrom
Fraunhofer-AISEC:fix-6.3.11-2.1

Conversation

@georgbramm

Copy link
Copy Markdown

Closes Issue for Test 6.3.11 in csaf version 2.1

@georgbramm georgbramm requested a review from a team as a code owner June 22, 2026 10:36
@tschmidtb51

tschmidtb51 commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

@georgbramm Please at least mention in the corresponding issue before you start that you want to work on that issue. So we can avoid doing the same work twice.

@aschierl-xitaso

aschierl-xitaso commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Before enabling an existing 2.0 test for 2.1, we typically

  • check if there are significant changes between the test specification in 2.0 and 2.1, and if so implement them, so that we can validate both 2.0 and 2.1 documents correctly
  • check if the existing implementation fulfills the specification, especially looking at reporting multiple errors in one test (no shortcut / fail fast)
  • check if all significant decisions are covered by the test files provided with the standard, and write custom test files to reach good coverage otherwise
  • run the validator binary on provided test files, and check if the validation errors (and corresponding JSON Pointers) are correct and helpful / match the result JSON if present

If all these conditions are met, we enable the test, and mark it as implemented in the Readme.md.

@tziemek tziemek linked an issue Jun 23, 2026 that may be closed by this pull request
4 tasks
@peinjoh

peinjoh commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

The test did not change between CSAF 2.0 and 2.1 and was to my understanding not touched by any bigger semantics changes, so the implementation should be fine. I also did not find any obvious issues, fail-fast, ...

For the code coverage, I would argue that some work needs to be done. There is only a passing test case with product version 4.2 and a failing test case with v4.2.

The case of a) V being uppercase (like V4.2), which should fail b) not being followed by an ASCII number (like vFOOBAR), which should pass, is not considered. The current test coverage would allow a trivial impl with startsWith('v') to pass the test suite.

Also, there is no test coverage for multiple error cases, so a fail-fast approach vs. exhaustive error search would also pass the test suite.

Also, there is no test coverage of the branch category check existing. i.e. that an architecture namend v4.2 should pass.

Also, and this might be in the realm of premature optimization: This regex is so simple we could try to do this with string ops, as in: test fails if string length >=2, first char is ignore-case 'v', second char is ascii digit. Even if we keep the regex, we can simplify from the standards recommendation of ^[vV][0-9].*$ to ^[vV][0-9] so the eval can stop earlier.

@georgbramm if you agree the impl looks fine, could you look into the perf optimization? Also, I'm opening a "Improve 6.3.11 test coverage" issue. Can interest you in creating the supplemental test cases (and others, if you see room for more) for this?

@peinjoh

peinjoh commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Before enabling an existing 2.0 test for 2.1, we typically

* check if there are significant changes between the test specification in 2.0 and 2.1, and if so implement them, so that we can validate both 2.0 and 2.1 documents correctly

* check if the existing implementation fulfills the specification, especially looking at reporting multiple errors in one test (no shortcut / fail fast)

* check if all significant decisions are covered by the test files provided with the standard, and write custom test files to reach good coverage otherwise

If all these conditions are met, we enable the test, and mark it as implemented in the Readme.md.

Also, pointing #700 to this well-written summary

@georgbramm georgbramm closed this Jul 16, 2026
@georgbramm georgbramm deleted the fix-6.3.11-2.1 branch July 16, 2026 14:56
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.

Implement 6.3.11 Usage of V as Version Indicator for CSAF 2.1

4 participants