elharo opened a new issue, #380:
URL: https://github.com/apache/maven-help-plugin/issues/380
### Affected version
HEAD
### Bug description
Bug Analysis for maven-help-plugin
HIGH Severity
1. EffectivePomMojo.cleanModel() mutates the live project's Model in-place
(EffectivePomMojo.java:210-213)
private static void cleanModel(Model pom) {
Properties properties = new SortedProperties();
properties.putAll(pom.getProperties());
pom.setProperties(properties); // <-- modifies the real project model
}
Called from writeEffectivePom() at line 182 on project.getModel(). This
permanently replaces the project's Properties object with a SortedProperties
instance, altering property iteration order for the remainder of the build. If
any subsequent mojo or plugin depends on insertion order or object identity of
Properties, it will break. This is a data corruption side effect in what should
be a read-only display operation.
2. EffectiveSettingsMojo mutates the original Settings object when
showPasswords=true (EffectiveSettingsMojo.java:87-94)
```
if (showPasswords) {
copySettings = settings; // <-- uses original, not a copy
} else {
copySettings = copySettings(settings);
...
}
// ...
writeEffectiveSettings(copySettings, writer); // calls cleanSettings() which
modifies profiles
```
When -DshowPasswords=true, the original Maven Settings object is used
directly. writeEffectiveSettings -> cleanSettings() then replaces each
profile's Properties with a SortedProperties instance (same pattern as bug #1).
This mutates the global shared settings object, corrupting state for subsequent
lifecycle steps.
3. DescribeMojo.lookupPluginDescriptor() error message references null mojo
fields instead of PluginInfo (DescribeMojo.java:333-338)
throw new MojoExecutionException(
"Error retrieving plugin descriptor for:" + LS + LS + "groupId: '"
+ groupId + "'" + LS + "artifactId: '" + artifactId + "'" + LS +
"version: '" + version + "'" + LS + LS, e);
When the plugin was specified by prefix (e.g., -Dplugin=help), the mojo
fields groupId, artifactId, version are all null. The method parameter
PluginInfo pi contains the actual values (pi.getGroupId(), etc.) but the error
message ignores them. Output: groupId: 'null'\nartifactId: 'null'\nversion:
'null' — a misleading error that defeats debugging.
MEDIUM Severity
4. AllProfilesMojo.addProjectPomProfiles() uses parent context for active
profiles (AllProfilesMojo.java:153-156)
if (project.getActiveProfiles() != null) {
for (Profile profile : project.getActiveProfiles()) {
activeProfiles.put(profile.getId(), profile);
}
}
project = project.getParent(); // walks up to parent
When the loop reaches a parent POM, project.getActiveProfiles() returns
profiles active in the parent's build context. These may not be active for the
child project (different JDK, OS, properties). The output will incorrectly mark
parent profiles as active when they aren't applicable to the child.
5. EffectiveSettingsMojo.copySettings() doesn't deep-copy profiles
(EffectiveSettingsMojo.java:156-198)
Only servers and proxies are manually deep-copied. The profiles list is
shared with the original via SettingsUtils.copySettings(). While
hidePasswords() only targets servers/proxies, profiles can also contain
sensitive data (passwords in profile properties) that could be exposed if the
original settings object is later queried expecting the copy to be isolated.
6. ListDependencyTypesMojo, ListLifecyclePhasesMojo, ListPackagingMojo write
to console AND file simultaneously (e.g., ListDependencyTypesMojo.java:95-96)
getLog().info(LS + "Maven Dependency Types defined:" + LS + LS +
descriptionBuffer);
writeFile(output, descriptionBuffer); // always called, even when output is
null
These three mojos always write to the log via getLog().info(...) and
unconditionally call writeFile(output, ...) (which is a no-op when output is
null). Every other mojo (ActiveProfilesMojo, AllProfilesMojo, DescribeMojo,
SystemMojo) uses if (output != null) { ... } else { getLog().info(...); }. The
dual-write is inconsistent and means content is always printed to console even
when writing to a file. They also don't prepend the "Generated by Maven Help
Plugin" header that ActiveProfilesMojo and SystemMojo add.
LOW Severity
7. DescribeMojo.toLines() delegates to generated HelpMojo via fragile
reflection (DescribeMojo.java:747-778)
Method m = HelpMojo.class.getDeclaredMethod("toLines", String.class,
Integer.TYPE, Integer.TYPE, Integer.TYPE);
m.setAccessible(true);
List<String> output = (List<String>) m.invoke(HelpMojo.class, text, indent,
indentSize, lineLength);
HelpMojo is generated by maven-plugin-tools at compile time (in
target/generated-sources/). It IS available at runtime, but this reflection is
fragile: any change in the generated code's toLines signature silently breaks
all describe output formatting. The HelpMojo.toLines internally calls repeat()
with new StringBuilder(repeat * str.length()) — a classic integer overflow risk
that could throw NegativeArraySizeException (caught, but still a brittle chain).
8. AbstractHelpMojo.LS uses System.getProperty("line.separator")
(AbstractHelpMojo.java:55)
protected static final String LS = System.getProperty("line.separator");
Should use System.lineSeparator() (Java 7+), which falls back to "\n" if the
property is missing. Additionally, DescribeMojoTest inconsistently uses both
System.getProperty("line.separator") (line 88) and System.lineSeparator() (line
110).
9. SortedProperties uses raw types (AbstractEffectiveMojo.java:147-148)
List list = new ArrayList(keynames);
Collections.sort(list);
The @SuppressWarnings({"rawtypes", "unchecked"}) annotation masks the issue
rather than using List<Object>.
10. AbstractHelpMojo.getAetherArtifact() uses deprecated
Artifact.LATEST_VERSION (AbstractHelpMojo.java:147)
version = Artifact.LATEST_VERSION;
Deprecated in favor of null (which means "latest") in the Aether API.
11. EffectivePomMojo encoding fallback can return null
(EffectivePomMojo.java:111)
String encoding = output != null ? project.getModel().getModelEncoding() :
System.getProperty("file.encoding");
System.getProperty("file.encoding") can theoretically return null.
12. AllProfilesMojo — settings profiles added to allProfilesByIds but never
tracked for active status (AllProfilesMojo.java:167-173)
addSettingsProfiles() only populates allProfilesByIds, never
activeProfilesByIds. Active detection relies on project.getActiveProfiles()
which may or may not include converted settings profiles depending on Maven
version and activation mechanisms. This can under-report active settings
profiles.
--
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]
