Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Improve case-sensitive handling #550

Merged
merged 2 commits into from
Apr 15, 2024

Conversation

DaveTryon
Copy link
Contributor

As called out in #523, we have some issues if a manifest file contains files whose names vary only by case. When this happens, we create a NullReferenceException that causes the validator to terminate prematurely without providing information to allow the person to address the issue. This PR does the following:

It leaves the overall validation logic unchanged, but it detects the case that was causing the NullReferenceException and reports it as an AdditionalFile error. This is consistent with the effect of copying 2 files whose names vary only by case to a file case-insensitive drive. This change allows the validation to complete with errors, rather than exiting prematurely

If errors are found when scanning on a case-insensitive system, it adds a brief warning to inform the users that if they created the manifest on a case-sensitive system, they may need to validate it on a case-sensitive system. We already have a mechanism to identify case-sensitivity, but it was private inside a class. It is now exposed on the associated interface. Unit tests have been updated to account for the case sensitivity check.

I simulated this case locally by generating a manifest, then manually creating some cases where file entries differed by case and by hash. When the validator runs, the order in which the manifest entries is processed is nondeterministic. As a result, the same manifest file can produce different errors. Depending on the order of processing, the errors may be reported as invalid hashes or as additional files. Because of this, I made the case sensitivity warning general and displayed it at the top of the output results. If any errors are found when validation is performed on a case-insensitive OS, the error header now reads as follows:

------------------------------------------------------------
Individual file validation results
  Note: If the manifest file was originally created using a
        case-sensitive OS, you may also need to validate it
        using a case-sensitive OS.
------------------------------------------------------------

There was also a private member in the OSUtils class that was not readonly and the compiler complained about it. I fixed it while I was touching the file.

@DaveTryon DaveTryon requested a review from a team as a code owner April 12, 2024 20:56
@DaveTryon DaveTryon requested review from ByAgenT and edgarrs April 12, 2024 20:56
@codecov-commenter
Copy link

codecov-commenter commented Apr 12, 2024

Codecov Report

Attention: Patch coverage is 37.50000% with 10 lines in your changes are missing coverage. Please review.

Project coverage is 59.60%. Comparing base (44dc753) to head (887d2e1).

Files Patch % Lines
...osoft.Sbom.Api/Workflows/Helpers/FilesValidator.cs 0.00% 7 Missing and 1 partial ⚠️
...Api/Workflows/SBOMParserBasedValidationWorkflow.cs 75.00% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #550      +/-   ##
==========================================
- Coverage   59.66%   59.60%   -0.06%     
==========================================
  Files         248      248              
  Lines        7581     7595      +14     
  Branches      902      905       +3     
==========================================
+ Hits         4523     4527       +4     
- Misses       2645     2652       +7     
- Partials      413      416       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@DaveTryon DaveTryon merged commit dc4e2e0 into microsoft:main Apr 15, 2024
6 checks passed
@DaveTryon DaveTryon deleted the improve-case-sensitive-handling branch April 15, 2024 21:53
tarun06 pushed a commit to tarun06/sbom-tool that referenced this pull request Jul 21, 2024
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.

3 participants