fix: parse CPE names correctly #4041 #4063
Conversation
Signed-off-by: fthdrmzzz <mail.fatih.durmaz@gmail.com>
| @@ -183,7 +183,8 @@ def parse_node(self, node: dict[str, list[dict[str, str]]]) -> list[dict[str, st | |||
| if "cpe_match" in node: | |||
| vulnerable_matches = (m for m in node["cpe_match"] if m["vulnerable"]) | |||
| for cpe_match in vulnerable_matches: | |||
| cpe_split = cpe_match["cpe23Uri"].split(":") | |||
| cpe_uri = cpe_match["cpe23Uri"] | |||
| cpe_split = re.split(r"(?<!\\):", cpe_uri) | |||
There was a problem hiding this comment.
here regex pattern splits with :'s only if there is no preceding \\.
There was a problem hiding this comment.
Minor change: If it's worth putting a comment in the PR, it's probably worth putting the comment in the code! (especially since regexes can be a pain to read) And I condensed things into a single line because I felt like it went better with the comment, though that's an entirely subjective choice.
Since these changes shouldn't make any functional diference, I'm going to merge them and let the tests re-run and this will probably be approved thereafter.
Also, this is making me wonder if we have a similar issue in the SBOM CPE code...
|
Ah, nevermind, looks like you didn't grant me permission to merge to your branch. So I'll leave merging the suggestions to you! But I want to say again that this looks great and thank you very much for spotting this issue and fixing it! |
Co-authored-by: Terri Oda <terri@toybox.ca>
Co-authored-by: Terri Oda <terri@toybox.ca>
Thank you for having time to review it quickly. I have merged your suggestions. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4063 +/- ##
==========================================
+ Coverage 75.41% 81.97% +6.56%
==========================================
Files 808 820 +12
Lines 11983 12533 +550
Branches 1598 1941 +343
==========================================
+ Hits 9037 10274 +1237
+ Misses 2593 1844 -749
- Partials 353 415 +62
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Merge time! Thank you again, and congratulations on your first merged commit with us @fthdrmzzz !
This PR fixes issue #4041. I have parsed cpe names with regex to basically escape `:``s in the cpe name.