gnodet-bot commented on code in PR #400:
URL: https://github.com/apache/maven-filtering/pull/400#discussion_r4103257897
##########
src/main/java/org/apache/maven/shared/filtering/BaseFilter.java:
##########
@@ -173,6 +177,68 @@ 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 searchRoot = prefix.length() > 0
+ ? FilteringUtils.resolveFile(basedir, prefix.toString())
Review Comment:
**Path traversal concern**: `FilteringUtils.resolveFile()` resolves `..` but
does not verify the result stays within `basedir`. A pattern like
`../../../etc/*.properties` would set `searchRoot` to a directory far outside
the project.
Consider adding a guard:
```java
Path normalizedBase = basedir.toAbsolutePath().normalize();
if (!searchRoot.startsWith(normalizedBase)) {
throw new MavenFilteringException(
"Filter glob pattern '" + globPattern + "' resolves outside project
basedir");
}
```
##########
src/main/java/org/apache/maven/shared/filtering/BaseFilter.java:
##########
@@ -189,10 +255,23 @@ void loadProperties(
continue;
}
try {
- Path propFile = FilteringUtils.resolveFile(basedir,
filterFile);
- Properties properties =
PropertyUtils.loadPropertyFile(propFile, workProperties, getLogger());
- filterProperties.putAll(properties);
- workProperties.putAll(properties);
+ if (isGlobPattern(filterFile)) {
+ List<Path> matched = expandGlob(basedir, filterFile);
+ if (matched.isEmpty()) {
+ getLogger().warn("Filter glob '" + filterFile + "'
did not match any files");
Review Comment:
Since `getLogger()` returns SLF4J `Logger`, prefer parameterized logging
over string concatenation:
```java
getLogger().warn("Filter glob '{}' did not match any files", filterFile);
```
This avoids the concatenation cost when warn-level logging is disabled.
##########
src/main/java/org/apache/maven/shared/filtering/BaseFilter.java:
##########
@@ -173,6 +177,68 @@ 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) {
Review Comment:
**Backward compatibility**: `isGlobPattern` triggers on any `*`, `?`, `{`,
or `[` in the path. File names like `filter[1].properties` or
`cache.{backup}.properties` are valid on Unix/macOS and would now be silently
misinterpreted as globs instead of loaded as literal files.
Consider a fallback: if the glob matches nothing but a literal file exists
at the path, load it literally. Or require an explicit opt-in prefix (e.g.
`glob:env/dev/*.properties`).
--
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]
