gnodet-bot commented on code in PR #400:
URL: https://github.com/apache/maven-filtering/pull/400#discussion_r4103423935
##########
src/main/java/org/apache/maven/shared/filtering/BaseFilter.java:
##########
@@ -173,6 +177,71 @@ public List<FilterWrapper> getDefaultFilterWrappers(final
AbstractMavenFiltering
return defaultFilterWrappers;
}
+ /**
+ * Returns {@code true} if the given filter path contains glob pattern
characters
+ * ({@code *}, {@code ?}, {@code {}, or {@code [}).
Review Comment:
💡 **Still unaddressed from previous review:** `{@code {}` is broken Javadoc
— the first `}` closes the `{@code` tag, so the rendered output loses the `{`
and leaves a stray `}`.
```suggestion
* ({@code *}, {@code ?}, <code>{</code>, or {@code [}).
```
##########
src/main/java/org/apache/maven/shared/filtering/BaseFilter.java:
##########
@@ -173,6 +177,71 @@ public List<FilterWrapper> getDefaultFilterWrappers(final
AbstractMavenFiltering
return defaultFilterWrappers;
}
+ /**
+ * Returns {@code true} if the given filter path contains glob pattern
characters
+ * ({@code *}, {@code ?}, {@code {}, or {@code [}).
+ */
+ private static boolean isGlobPattern(String path) {
+ return path.indexOf('*') >= 0 || path.indexOf('?') >= 0 ||
path.indexOf('{') >= 0 || path.indexOf('[') >= 0;
+ }
+
+ /**
+ * Expands a glob pattern relative to {@code basedir} and returns the
matched paths in sorted order.
+ * The pattern must use forward slashes as path separators (as is
conventional in Maven filter paths).
+ */
+ private List<Path> expandGlob(Path basedir, String globPattern) throws
IOException {
+ // Normalize to forward slashes; resolveFile accepts them on all
platforms
+ String normalized = globPattern.replace('\\', '/');
+
+ // Find the deepest non-glob path prefix to use as the walk root
+ String[] segments = normalized.split("/");
+ StringBuilder prefix = new StringBuilder();
+ for (String segment : segments) {
+ if (segment.indexOf('*') >= 0
+ || segment.indexOf('?') >= 0
+ || segment.indexOf('{') >= 0
+ || segment.indexOf('[') >= 0) {
+ break;
+ }
+ if (prefix.length() > 0) {
+ prefix.append('/');
+ }
+ prefix.append(segment);
+ }
+
+ Path normalizedBase = basedir.toAbsolutePath().normalize();
+ Path searchRoot = prefix.length() > 0 ?
FilteringUtils.resolveFile(basedir, prefix.toString()) : normalizedBase;
+
+ if (!searchRoot.startsWith(normalizedBase)) {
+ throw new IOException("Filter glob pattern '" + globPattern + "'
resolves outside project basedir");
+ }
+
+ if (!Files.isDirectory(searchRoot)) {
+ return List.of();
+ }
+
+ // Build a relative glob pattern for the suffix beyond the prefix.
+ // We match against the path relative to searchRoot to avoid OS
separator issues:
+ // on Windows, Path.toString() uses '\' which glob treats as an escape
character.
+ // Relative matching with forward-slash patterns works on all
platforms because
+ // Path.relativize() always returns paths comparable with '/' in glob
patterns.
+ String patternSuffix = prefix.length() > 0 ?
normalized.substring(prefix.length() + 1) : normalized;
+ PathMatcher matcher = FileSystems.getDefault().getPathMatcher("glob:"
+ patternSuffix);
+
+ List<Path> matched = new ArrayList<>();
+ try (Stream<Path> stream = Files.walk(searchRoot)) {
Review Comment:
💡 **Still unaddressed from previous review:** `Files.walk()` without
`maxDepth` traverses the entire subtree under `searchRoot`. For a single-`*`
pattern like `env/dev/*.properties`, the `PathMatcher` correctly rejects deep
files but the walker still visits every subdirectory — wasteful I/O on large
trees.
```suggestion
try (Stream<Path> stream = Files.walk(searchRoot,
patternSuffix.contains("/") || patternSuffix.contains("**") ? Integer.MAX_VALUE
: 1)) {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
