gnodet commented on code in PR #610:
URL: https://github.com/apache/maven-jar-plugin/pull/610#discussion_r4224522151
##########
src/main/java/org/apache/maven/plugins/jar/ToolExecutor.java:
##########
@@ -69,6 +70,104 @@ final class ToolExecutor {
*/
private static final String CREATED_BY = "Created-By";
+ /**
+ * Matches any character that is not an ASCII letter or digit.
+ * Used by {@link #cleanModuleName(String)} to mirror the JDK's {@code
ModulePath.cleanModuleName()} algorithm.
+ */
+ private static final Pattern NON_ALPHANUM =
Pattern.compile("[^A-Za-z0-9]");
+
+ /**
+ * Matches two or more consecutive dots.
+ * Used by {@link #cleanModuleName(String)} to collapse repeated
separators.
+ */
+ private static final Pattern REPEATING_DOTS = Pattern.compile("\\.{2,}");
+
+ /**
+ * Sanitizes a candidate automatic module name by applying the same
algorithm that the JDK uses
+ * to derive an automatic module name from a JAR file name
+ * (see {@code jdk.internal.module.ModulePath.cleanModuleName()}):
+ * non-alphanumeric characters (including hyphens) are replaced with
{@code '.'}, repeated dots
+ * are collapsed to a single dot, and leading/trailing dots are stripped.
+ *
+ * @param name the raw, potentially invalid module name
+ * @return the sanitized name, or an empty string if nothing remains after
sanitization
+ */
+ private static String cleanModuleName(String name) {
+ name = NON_ALPHANUM.matcher(name).replaceAll(".");
+ name = REPEATING_DOTS.matcher(name).replaceAll(".");
+ int len = name.length();
+ if (len > 0 && name.charAt(0) == '.') {
+ name = name.replaceFirst("^\\.+", "");
+ }
+ len = name.length();
+ if (len > 0 && name.charAt(len - 1) == '.') {
+ name = name.replaceFirst("\\.+$", "");
+ }
+ return name;
+ }
+
+ /**
+ * Validates the {@code Automatic-Module-Name} attribute in the given
manifest.
+ * If the attribute is absent or already valid, this method does nothing
and returns {@code false}.
+ *
+ * <p>If the name is invalid and was explicitly declared in the POM via
+ * {@code <archive><manifestEntries><Automatic-Module-Name>}, the build
fails immediately
+ * (the developer chose that name and must fix it).</p>
+ *
+ * <p>If the name is invalid and was read from a {@code MANIFEST.MF} file
(e.g. from the
+ * compiled output directory), the {@linkplain #cleanModuleName(String)
JDK sanitization
+ * algorithm} is applied and a warning is logged. If the sanitized name
is still invalid
+ * the attribute is removed and a warning is logged (MJAR-596).</p>
+ *
+ * @param manifest the merged manifest to inspect and potentially modify
+ * @return {@code true} if the manifest was modified and a temporary
manifest file must be written
+ * @throws MojoException if the name was explicitly declared in POM
configuration and is invalid
+ */
+ private boolean sanitizeAutomaticModuleName(Manifest manifest) {
+ String name =
manifest.getMainAttributes().getValue("Automatic-Module-Name");
+ if (name == null || SourceVersion.isName(name)) {
+ return false;
+ }
+ /*
+ * If the invalid name originates from an explicit
<archive><manifestEntries> declaration
+ * in the POM, fail the build so that the developer is alerted
immediately — they set an
+ * invalid name on purpose and should fix it. This matches the
historical maven-archiver
+ * behaviour (MJAR-260).
+ *
+ * If the name was read from a MANIFEST.MF file (either from the
compiled output directory
+ * or from <archive><manifestFile>), apply the same sanitization
algorithm the JDK uses
+ * when deriving an automatic module name from a JAR filename
Review Comment:
Good question. The distinction we're making is:
- **POM `<manifestEntries>`**: the developer explicitly authored that name →
fail fast so they fix it (MJAR-260 behaviour).
- **`MANIFEST.MF` resource** (either from `src/main/resources/META-INF/` or
`<archive><manifestFile>`): the file is often part of a dependency JAR being
repackaged, or generated by a build step the developer doesn't directly
control. Failing the build in that case would be surprising — the developer may
not even know the file exists. The JDK itself applies sanitization rather than
failing when it encounters an invalid name in a JAR file name. Dropping the
attribute with a warning follows the same principle: give the build a chance to
succeed while making the problem visible.
That said, if a sanitized name cannot be produced at all (empty result),
removing the attribute silently enough is the conservative choice — the
alternative (a totally broken module name) is worse for consumers.
--
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]
