Skip to content

Commit adf3b10

Browse files
a-orenclaude
andcommitted
fix: address code review feedback for requirements.txt preprocessing
- Revert getIgnoredDependencies to use raw lines so ignore markers (e.g. #trustify-da-ignore) in inline comments are not stripped before containsIgnorePattern runs; keep \R split fix - Use compiled Pattern with \s-- for inline option stripping to handle tabs and multiple spaces, not just single space - Add Windows drive-letter path filtering (C:\path, c:/path) - Tighten :// URL check to only inspect the requirement part before markers, avoiding false positives from marker strings - Add tests for Windows paths and final-line backslash edge case Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 3dc259a commit adf3b10

3 files changed

Lines changed: 38 additions & 10 deletions

File tree

src/main/java/io/github/guacsec/trustifyda/providers/PythonPipProvider.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -95,9 +95,8 @@ public Content provideComponent() throws IOException {
9595

9696
@Override
9797
protected Set<PackageURL> getIgnoredDependencies(String manifestContent) {
98-
List<String> rawLines = Arrays.asList(manifestContent.split("\\R"));
99-
List<String> preprocessed = PythonControllerBase.preprocessRequirementsLines(rawLines);
100-
return preprocessed.stream()
98+
String[] lines = manifestContent.split("\\R");
99+
return Arrays.stream(lines)
101100
.filter(this::containsIgnorePattern)
102101
.map(PythonPipProvider::extractDepFull)
103102
.map(this::splitToNameVersion)

src/main/java/io/github/guacsec/trustifyda/utils/PythonControllerBase.java

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@
3535
import java.util.List;
3636
import java.util.Map;
3737
import java.util.logging.Logger;
38+
import java.util.regex.Matcher;
39+
import java.util.regex.Pattern;
3840
import java.util.stream.Collectors;
3941
import java.util.stream.Stream;
4042

@@ -372,6 +374,9 @@ protected String getDependencyNameShow(String pipShowOutput) {
372374
return versionToken.substring(0, endOfLine).trim();
373375
}
374376

377+
private static final Pattern INLINE_OPTION_PATTERN = Pattern.compile("\\s--");
378+
private static final Pattern WINDOWS_DRIVE_PATH_PATTERN = Pattern.compile("^[a-zA-Z]:[/\\\\]");
379+
375380
/**
376381
* Preprocesses raw requirements.txt lines by joining line continuations, stripping inline
377382
* options, and filtering out pip option lines, URLs, local paths, and empty/comment lines.
@@ -404,8 +409,11 @@ public static List<String> preprocessRequirementsLines(List<String> rawLines) {
404409
if (line.startsWith("-")) {
405410
continue;
406411
}
407-
// Filter out local path requirements (./path, ../path, /abs/path)
408-
if (line.startsWith("./") || line.startsWith("../") || line.startsWith("/")) {
412+
// Filter out local path requirements (./path, ../path, /abs/path, C:\path, C:/path)
413+
if (line.startsWith("./")
414+
|| line.startsWith("../")
415+
|| line.startsWith("/")
416+
|| WINDOWS_DRIVE_PATH_PATTERN.matcher(line).find()) {
409417
continue;
410418
}
411419
// Strip PEP 508 direct references (name @ url -> name) before URL check
@@ -414,12 +422,14 @@ public static List<String> preprocessRequirementsLines(List<String> rawLines) {
414422
line = line.substring(0, atIndex).trim();
415423
}
416424
// Strip inline pip options (--hash=..., --config-settings=..., etc.)
417-
int optionIndex = line.indexOf(" --");
418-
if (optionIndex != -1) {
419-
line = line.substring(0, optionIndex).trim();
425+
Matcher optionMatcher = INLINE_OPTION_PATTERN.matcher(line);
426+
if (optionMatcher.find()) {
427+
line = line.substring(0, optionMatcher.start()).trim();
420428
}
421-
// Filter out bare URLs and VCS URLs (any line containing :// is not a package name)
422-
if (line.contains("://")) {
429+
// Filter out bare URLs and VCS URLs — check the requirement part (before any marker)
430+
// to avoid false positives from marker strings
431+
String requirementPart = line.contains(";") ? line.substring(0, line.indexOf(";")) : line;
432+
if (requirementPart.contains("://")) {
423433
continue;
424434
}
425435
if (!line.isEmpty()) {

src/test/java/io/github/guacsec/trustifyda/utils/PythonControllerBaseTest.java

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2164,6 +2164,25 @@ void preprocessRequirementsLines_filters_local_paths() {
21642164
assertEquals(List.of("requests==2.28.0"), result);
21652165
}
21662166

2167+
@Test
2168+
void preprocessRequirementsLines_filters_windows_paths() {
2169+
List<String> input =
2170+
List.of(
2171+
"C:\\Users\\dev\\my-package",
2172+
"c:/projects/my-lib",
2173+
"D:\\packages\\MyPackage-1.0.tar.gz",
2174+
"requests==2.28.0");
2175+
List<String> result = PythonControllerBase.preprocessRequirementsLines(input);
2176+
assertEquals(List.of("requests==2.28.0"), result);
2177+
}
2178+
2179+
@Test
2180+
void preprocessRequirementsLines_handles_final_line_ending_with_backslash() {
2181+
List<String> input = List.of("flask==2.0.3", "requests==2.28.0 \\");
2182+
List<String> result = PythonControllerBase.preprocessRequirementsLines(input);
2183+
assertEquals(List.of("flask==2.0.3", "requests==2.28.0"), result);
2184+
}
2185+
21672186
@Test
21682187
void preprocessRequirementsLines_combined_scenario() {
21692188
List<String> input =

0 commit comments

Comments
 (0)