Fix #590 : Check mkdirs() result during WAR assembly - #618
Open
HarshMehta112 wants to merge 4 commits into
Open
Conversation
Signed-off-by: Harsh Mehta <harshmehta010102@gmail.com>
Signed-off-by: Harsh Mehta <harshmehta010102@gmail.com>
elharo
requested changes
Jul 4, 2026
| * @return the temp directory for the overlay | ||
| */ | ||
| protected File getOverlayTempDirectory(WarPackagingContext context, Overlay overlay) { | ||
| protected File getOverlayTempDirectory(WarPackagingContext context, Overlay overlay) throws MojoExecutionException { |
Contributor
There was a problem hiding this comment.
this is tricky because we don't want to change the signature here. maybe remove this file from the pR and come back to it in another PR
Author
There was a problem hiding this comment.
Hi @elharo
I’ve reverted the change as suggested. Just curious—could you help me understand the reason behind it? Are there any downstream dependencies (other plugins/Maven modules) that might be affected? This is for my understanding only, not a concern.
Contributor
There was a problem hiding this comment.
Public APIs only change in a major release with clearly advertised breaking changes. You do not know what the downstream dependencies are. Hyrum's Law applies.
Signed-off-by: Harsh Mehta <harshmehta010102@gmail.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses #590 by adding failure handling around directory creation during WAR/exploded webapp assembly, aiming to surface directory-creation problems earlier via MojoExecutionException.
Changes:
- Add
mkdirs()return-value checks when creatingWEB-INF/META-INFduring WAR project packaging. - Add
mkdirs()return-value checks when creatingWEB-INF/classes. - Add
mkdirs()return-value checks for parent directory creation before filtered resource copies and for exploded webapp root creation.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
src/main/java/org/apache/maven/plugins/war/packaging/WarProjectPackagingTask.java |
Adds failure checks when creating WEB-INF and META-INF directories. |
src/main/java/org/apache/maven/plugins/war/packaging/ClassesPackagingTask.java |
Adds failure check when creating the WEB-INF/classes directory. |
src/main/java/org/apache/maven/plugins/war/packaging/AbstractWarPackagingTask.java |
Adds failure check when creating parent directories before copying filtered files. |
src/main/java/org/apache/maven/plugins/war/AbstractWarMojo.java |
Adds failure check when creating the exploded webapp root directory. |
Comments suppressed due to low confidence (1)
src/main/java/org/apache/maven/plugins/war/packaging/WarProjectPackagingTask.java:88
- Same directory-creation issue as above: exists() doesn’t verify that META-INF is actually a directory (it could be a file). Use isDirectory() and re-check after mkdirs() to fail early with a clear MojoExecutionException.
File metainfDir = new File(context.getWebappDirectory(), META_INF_PATH);
if (!metainfDir.exists() && !metainfDir.mkdirs()) {
throw new MojoExecutionException("Failed to create directory: " + metainfDir.getAbsolutePath());
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
81
to
+84
| File webinfDir = new File(context.getWebappDirectory(), WEB_INF_PATH); | ||
| webinfDir.mkdirs(); | ||
| if (!webinfDir.exists() && !webinfDir.mkdirs()) { | ||
| throw new MojoExecutionException("Failed to create directory: " + webinfDir.getAbsolutePath()); | ||
| } |
Comment on lines
54
to
57
| final File webappClassesDirectory = new File(context.getWebappDirectory(), CLASSES_PATH); | ||
| if (!webappClassesDirectory.exists()) { | ||
| webappClassesDirectory.mkdirs(); | ||
| if (!webappClassesDirectory.exists() && !webappClassesDirectory.mkdirs()) { | ||
| throw new MojoExecutionException("Failed to create directory: " + webappClassesDirectory.getAbsolutePath()); | ||
| } |
Comment on lines
+257
to
+260
| File parentDir = targetFile.getParentFile(); | ||
| if (!parentDir.exists() && !parentDir.mkdirs()) { | ||
| throw new MojoExecutionException("Failed to create directory: " + parentDir.getAbsolutePath()); | ||
| } |
Comment on lines
+462
to
+465
| if (!webapplicationDirectory.exists() && !webapplicationDirectory.mkdirs()) { | ||
| throw new MojoExecutionException( | ||
| "Failed to create directory: " + webapplicationDirectory.getAbsolutePath()); | ||
| } |
Comment on lines
80
to
+83
| // Prepare the INF directories | ||
| File webinfDir = new File(context.getWebappDirectory(), WEB_INF_PATH); | ||
| webinfDir.mkdirs(); | ||
| if (!webinfDir.exists() && !webinfDir.mkdirs()) { | ||
| throw new MojoExecutionException("Failed to create directory: " + webinfDir.getAbsolutePath()); |
Comment on lines
+82
to
+83
| if (!webinfDir.exists() && !webinfDir.mkdirs()) { | ||
| throw new MojoExecutionException("Failed to create directory: " + webinfDir.getAbsolutePath()); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes : #590
This change adds proper validation for directory creation by checking the return value of
mkdirs()and throwing aMojoExecutionExceptionif the directory cannot be created.Following this checklist to help us incorporate your
contribution quickly and easily:
Note that commits might be squashed by a maintainer on merge.
This may not always be possible but is a best-practice.
mvn verifyto make sure basic checks pass.A more thorough check will be performed on your pull request automatically.
mvn -Prun-its verify).If your pull request is about ~20 lines of code you don't need to sign an
Individual Contributor License Agreement if you are unsure
please ask on the developers list.
To make clear that you license your contribution under
the Apache License Version 2.0, January 2004
you have to acknowledge this by using the following check-box.