Copilot commented on code in PR #308:
URL:
https://github.com/apache/maven-remote-resources-plugin/pull/308#discussion_r4229712758
##########
src/main/java/org/apache/maven/plugin/resources/remote/AbstractProcessRemoteResourcesMojo.java:
##########
@@ -992,6 +992,20 @@ protected void processResourceBundles(ClassLoader
classLoader, VelocityContext c
}
}
+ static File resolveOutputFile(File outputDirectory, String resourceName)
+ throws IOException, MojoExecutionException {
+ if (new File(resourceName).isAbsolute()) {
+ throw new MojoExecutionException("Remote resource is outside the
output directory: " + resourceName);
+ }
+ File canonicalOutputDirectory = outputDirectory.getCanonicalFile();
+ File outputFile = new File(outputDirectory,
resourceName).getCanonicalFile();
+ if (outputFile.equals(canonicalOutputDirectory)
+ ||
!outputFile.toPath().startsWith(canonicalOutputDirectory.toPath())) {
+ throw new MojoExecutionException("Remote resource is outside the
output directory: " + resourceName);
+ }
+ return outputFile;
+ }
Review Comment:
resolveOutputFile() recomputes outputDirectory.getCanonicalFile() on every
call. Since this is invoked per resource during processing, it can introduce
avoidable filesystem/canonicalization overhead for bundles with many entries.
Consider computing the canonical output directory (or its Path) once per
execution/processing loop and passing it into the resolver (e.g., overload
resolveOutputFile to accept a precomputed canonical/Path) so only the resource
path is canonicalized per entry.
##########
src/main/java/org/apache/maven/plugin/resources/remote/AbstractProcessRemoteResourcesMojo.java:
##########
@@ -992,6 +992,20 @@ protected void processResourceBundles(ClassLoader
classLoader, VelocityContext c
}
}
+ static File resolveOutputFile(File outputDirectory, String resourceName)
+ throws IOException, MojoExecutionException {
+ if (new File(resourceName).isAbsolute()) {
+ throw new MojoExecutionException("Remote resource is outside the
output directory: " + resourceName);
+ }
+ File canonicalOutputDirectory = outputDirectory.getCanonicalFile();
+ File outputFile = new File(outputDirectory,
resourceName).getCanonicalFile();
+ if (outputFile.equals(canonicalOutputDirectory)
+ ||
!outputFile.toPath().startsWith(canonicalOutputDirectory.toPath())) {
+ throw new MojoExecutionException("Remote resource is outside the
output directory: " + resourceName);
Review Comment:
The exception message is used for both (1) absolute path rejection and (2)
canonical path escape, but the text says the resource is 'outside the output
directory' even when the real issue is simply that absolute paths are not
allowed. Consider rewording to something like 'Remote resource path must be
relative and within the output directory' (and optionally include the
outputDirectory in the message). Also, the message string is duplicated in two
places—centralizing it (constant or a small helper) helps keep the wording
consistent.
--
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]
