Skip to content

get_relative_path: shorten data_folder everywhere it appears#86

Open
abrignoni wants to merge 1 commit into
mainfrom
relative-paths-concatenated
Open

get_relative_path: shorten data_folder everywhere it appears#86
abrignoni wants to merge 1 commit into
mainfrom
relative-paths-concatenated

Conversation

@abrignoni

Copy link
Copy Markdown
Owner

Follow-up to the relative-source-paths fix, prompted by @JamesHabben's question about concatenated source paths.

The gap: the decorator shortens source_path per newline segment. Artifacts that join multiple absolute paths with other separators (', ', '; ', ' ') only had the first path shortened — startswith matches the head of the concatenated string, strips one prefix, and the rest stayed absolute:

in : <data>/a.db, <data>/b.db
out: a.db, /Users/examiner/.../b.db   <- second path leaked

The fix: Context.get_relative_path now globally replaces the data_folder prefix wherever it appears, separator-agnostic — same spirit as the old file_found.replace(seeker.data_folder, '') idiom. Artifacts that already relativize before joining (Oops-style, ~24 in iLEAPP) are unaffected: relative segments pass through untouched.

Verified: unit tests cover exact path, path == data_folder, literals, already-relative, None, comma/semicolon/space-joined absolute paths, newline decorator path, and no-data_folder passthrough. pylint 10.00/10, byte-compile, PluginLoader smoke test all pass.

Affected artifacts that this transparently fixes: iLEAPP box/home_depot/idstatuscache/sysdiagnose (+skg_archive if its parts are paths), ALEAPP FCM family/siminfo/wifiConfigstore2/wifiProfiles. RLEAPP/VLEAPP have no current emitters — this keeps the four frameworks identical and future-proof.

The relative-source-paths fix shortened the returned source_path per
newline segment, but artifacts that join multiple absolute paths with
other separators (', ', '; ', ' ') only had the first path shortened:
startswith matches the head of the concatenated string, strips one
prefix, and the rest stayed absolute. Spotted by James Habben.

Replace the startswith branch with a global replacement of the
data_folder prefix, so every embedded occurrence is shortened regardless
of separator. All previous behaviors are preserved (exact path, literal
passthrough, already-relative, no data_folder) and covered by tests.
@abrignoni

Copy link
Copy Markdown
Owner Author

Verification summary

Vetted this change against current main before merge. Findings:

Behavior is correct. Ran the old vs. new get_relative_path logic across 9 cases with base = /data/extract:

case OLD NEW
empty '' ''
exact match '' ''
normal base/a/b.txt a/b.txt a/b.txt
comma-join base/a.txt, base/b.txt a.txt, /data/extract/b.txt a.txt, b.txt
semicolon-join a.txt; /data/extract/b.txt a.txt; b.txt
space-join a.txt /data/extract/b.txt a.txt b.txt
no data_folder (passthrough) /other/x.txt /other/x.txt
already relative already/relative.txt already/relative.txt
windows sep base\a\b.txt a\b.txt a\b.txt

The six non-concatenation cases are byte-identical old vs. new; only the three genuinely-broken joined-path cases change, which is exactly the intent. Complements the existing per-newline shortening in the artifact_processor wrapper (which splits source_path on \n and calls this per segment) by also handling the , / ; / separators used inside a single segment.

Mechanics: current main's get_relative_path matches this PR's "before" byte-for-byte; GitHub reports MERGEABLE / CLEAN; single commit, one file, py_compile clean.

Two notes for the merger:

  • The commit message says "covered by tests," but there are no test files in the repo — the table above is a manual stand-in, not automated coverage.
  • The new logic uses base in full_path + .replace(...) instead of startswith + slice, so it's marginally more aggressive: a bare data_folder substring appearing as non-path content would also be stripped. In practice data_folder is an absolute extraction root that won't collide with real data, and the old startswith had the analogous /data vs /data2 prefix ambiguity, so this isn't a regression.

Touches core scripts/context.py, so leaving the merge decision to a human.

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.

1 participant