5.4.27#116
Merged
Merged
Conversation
Report the file path associated with each url hash in purl_scan output. The path is decrypted in handle_file_for_purls (as component_from_file does) and carried through to the url handler so each url_hash is paired with its path. url_hashes storage now keeps hash+path together so they stay aligned when sorted. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Drop latest, licenses, health, url_stats, dependencies, copyrights and vulnerabilities from the component detail. Only the allowlisted fields are reported: purls, vendor, component, version, url, release_date, file, rank. Also remove the now-unused file_md5_ref guard that only served the removed enrichers. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
-P help said "purls and versions" but the output has no version field; correct it and mention the new url hashes / source paths. Note -C accepts a comma-separated list. Update the purl_scan.c file header to reflect the oss_path field.
Apply the same version-stripping rule fill_component_path uses for the match path: expose look_for_version and run oss_path through it so a path like "libfoo-1.2.3/src/a.c" is reported as "src/a.c". Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
osadl_print_license() extracted the object between the first '{' and
first '}' after a matched key, assuming a flat license entry. When a
license token collided with a structural key (e.g. the top-level
"licenses" key whose value holds nested objects), the first '}' closed
an inner object, emitting an unbalanced '{' and breaking the JSON output.
Guard against a missing '{' and reject matches whose object contains a
nested '{', so only flat per-license entries are emitted.
📝 WalkthroughWalkthroughThis PR adds source-path tracking to purl URL hashes in ChangesPurl Scan URL Hash Path Tracking
Estimated code review effort: 3 (Moderate) | ~25 minutes OSADL License JSON Parsing Hardening
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant FileTableHandler
participant PurlScanCtx
participant UrlTableHandler
participant PurlEntry
FileTableHandler->>FileTableHandler: decrypt and normalize path via look_for_version
FileTableHandler->>PurlScanCtx: set current_path
FileTableHandler->>UrlTableHandler: trigger nested URL lookup
UrlTableHandler->>PurlEntry: add url_hash and current_path
FileTableHandler->>PurlScanCtx: clear current_path
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/license.c (1)
252-266: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReturn failure when the closing brace is missing.
When
strchr(content, '}')returns NULL, the nested-object check is skipped (end &&short-circuits), theif (end)extraction block is skipped, but the function does not return 0 — it falls through to print theosadl_updatedversion key. This produces output containing only the version with no license data, yetlen > 0causes the caller to treat it as a successful result ({"license": {"osadl_updated":"..."}}instead of{"license": {}}).For consistency with the new
if (!open) return 0guard at lines 248-249, add an early return whenendis NULL.🛡️ Proposed fix
char *end = strchr(content, '}'); /* Only accept flat license entries: if the object contains a nested object, this key is a structural one (e.g. the top-level "licenses" key), not a per-license entry, and copying it would emit unbalanced braces and break the JSON. */ if (end && memchr(content, '{', end - content)) return 0; + if (!end) + return 0; if (end)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/license.c` around lines 252 - 266, The license extraction path in src/license.c should fail fast when a closing brace is missing. In the logic around strchr(content, '}') and the subsequent nested-object and extraction checks, add an early return 0 when end is NULL so the function does not fall through to emit only the osadl_updated version key. Keep the behavior consistent with the existing open-brace guard and preserve the flat-license handling in this parsing block.src/purl_scan.c (1)
361-390: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep the
-CJSON schema stable atsrc/purl_scan.c:361-390. This path now dropslatestand the enrichment blocks (licenses,health,dependencies,copyrights,vulnerabilities), so downstream consumers will see a breaking shape change.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/purl_scan.c` around lines 361 - 390, The -C JSON output in purl_scan.c changed shape by removing fields that downstream consumers rely on. Update the output logic around fetch_related_purls, fill_main_url, and the JSON printing in the component serializer to keep the existing schema stable by restoring latest and the enrichment blocks for licenses, health, dependencies, copyrights, and vulnerabilities while preserving the current vendor/component/version/url/file/rank fields.
🧹 Nitpick comments (1)
src/purl_scan.c (1)
118-144: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winCheck
reallocandstrdupreturn values before use.If
reallocfails at line 134, the olde->url_hashespointer is leaked and the subsequent array access at lines 136-137 dereferences NULL. Similarly, if eitherstrdupfails, the entry gets a NULLhashorpath, which will crash later in the dedupstrcmp(line 128) or inscape_slashes(line 296).🛡️ Proposed fix: use a temporary pointer and validate allocations
static void purl_entry_add_url_hash(purl_entry_t *e, const char *url_hash, const char *path) { if (!url_hash || !*url_hash) return; for (int i = 0; i < e->n_url_hashes; i++) if (!strcmp(e->url_hashes[i].hash, url_hash)) return; if (e->n_url_hashes >= e->url_hashes_cap) { e->url_hashes_cap = e->url_hashes_cap ? e->url_hashes_cap * 2 : 8; - e->url_hashes = realloc(e->url_hashes, e->url_hashes_cap * sizeof(url_hash_t)); + url_hash_t *tmp = realloc(e->url_hashes, e->url_hashes_cap * sizeof(url_hash_t)); + if (!tmp) + return; + e->url_hashes = tmp; } - e->url_hashes[e->n_url_hashes].hash = strdup(url_hash); - e->url_hashes[e->n_url_hashes].path = strdup(path ? path : ""); + char *hash_copy = strdup(url_hash); + char *path_copy = strdup(path ? path : ""); + if (!hash_copy || !path_copy) + { + free(hash_copy); + free(path_copy); + return; + } + e->url_hashes[e->n_url_hashes].hash = hash_copy; + e->url_hashes[e->n_url_hashes].path = path_copy; e->n_url_hashes++; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/purl_scan.c` around lines 118 - 144, In purl_entry_add_url_hash, handle allocation failures instead of assuming realloc and strdup succeed. Use a temporary pointer for the growth of e->url_hashes so a failed realloc does not overwrite the original buffer, and only update e->url_hashes/e->url_hashes_cap after success. Also validate the strdup results before storing them in the url_hash_t entry; if either hash or path allocation fails, clean up and do not increment e->n_url_hashes so later strcmp in dedup and url handling in url_hash_cmp/scape_slashes never sees NULL fields.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/license.c`:
- Around line 252-266: The license extraction path in src/license.c should fail
fast when a closing brace is missing. In the logic around strchr(content, '}')
and the subsequent nested-object and extraction checks, add an early return 0
when end is NULL so the function does not fall through to emit only the
osadl_updated version key. Keep the behavior consistent with the existing
open-brace guard and preserve the flat-license handling in this parsing block.
In `@src/purl_scan.c`:
- Around line 361-390: The -C JSON output in purl_scan.c changed shape by
removing fields that downstream consumers rely on. Update the output logic
around fetch_related_purls, fill_main_url, and the JSON printing in the
component serializer to keep the existing schema stable by restoring latest and
the enrichment blocks for licenses, health, dependencies, copyrights, and
vulnerabilities while preserving the current
vendor/component/version/url/file/rank fields.
---
Nitpick comments:
In `@src/purl_scan.c`:
- Around line 118-144: In purl_entry_add_url_hash, handle allocation failures
instead of assuming realloc and strdup succeed. Use a temporary pointer for the
growth of e->url_hashes so a failed realloc does not overwrite the original
buffer, and only update e->url_hashes/e->url_hashes_cap after success. Also
validate the strdup results before storing them in the url_hash_t entry; if
either hash or path allocation fails, clean up and do not increment
e->n_url_hashes so later strcmp in dedup and url handling in
url_hash_cmp/scape_slashes never sees NULL fields.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c40f6d15-0e73-436b-bbd7-96835ece73e8
📒 Files selected for processing (5)
inc/component.hsrc/component.csrc/help.csrc/license.csrc/purl_scan.c
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
-C/--url-hashoption now accepts a single hash or a comma-separated list.Bug Fixes