You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "gnodet (via GitHub)" <gi...@apache.org> on 2026/09/25 18:25:21 UTC
[PR] docs: document null-value behaviour in manifest entry methods [maven-archiver]
gnodet opened a new pull request, #411:
URL: https://github.com/apache/maven-archiver/pull/411
## Summary
Addresses #374.
Null values passed as manifest attribute values are silently accepted and converted to empty strings by `sanitizeManifestValue()`. This is intentional behaviour (the existing test explicitly exercises it), but it can mask programming errors such as an unguarded map lookup returning `null`.
Rather than changing the behaviour, this PR documents the contract clearly on all the relevant public methods:
- `MavenArchiveConfiguration.addManifestEntry()`
- `MavenArchiveConfiguration.addManifestEntries()`
- `MavenArchiveConfiguration.getManifestEntries()`
- `MavenArchiveConfiguration.setManifestEntries()`
- `MavenArchiver.sanitizeManifestValue()` (private, for maintainers)
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] docs: document null-value behaviour in manifest entry methods [maven-archiver]
Posted by "gnodet-bot (via GitHub)" <gi...@apache.org>.
gnodet-bot commented on code in PR #411:
URL: https://github.com/apache/maven-archiver/pull/411#discussion_r4107900163
##########
src/main/java/org/apache/maven/shared/archiver/MavenArchiveConfiguration.java:
##########
@@ -152,19 +152,29 @@ public void setManifest(ManifestConfiguration manifest) {
}
/**
- * <p>addManifestEntry.</p>
+ * Adds a single manifest entry.
*
- * @param key the key of the entry
- * @param value the value of the entry
+ * <p>A {@code null} value is accepted and will produce an empty-string attribute in the manifest.
+ * This is intentional: it allows callers to explicitly request an empty manifest entry.
+ * Be careful not to pass {@code null} unintentionally (e.g., from an unguarded map lookup),
+ * as the resulting empty attribute will not indicate any error.</p>
+ *
+ * @param key the manifest attribute name
+ * @param value the manifest attribute value; {@code null} is treated as an empty string
Review Comment:
💡 The PR description says it documents the contract "on all the relevant public methods," but `ManifestSection` has the same `addManifestEntry(String, String)`, `addManifestEntries(Map)`, and `getManifestEntries()` methods that follow the exact same null-value path — values stored here are later sanitized via `sanitizeManifestValue()` in `MavenArchiver` (line 169). They should get the same Javadoc treatment for completeness.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] docs: document null-value behaviour in manifest entry methods [maven-archiver]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #411:
URL: https://github.com/apache/maven-archiver/pull/411#discussion_r4144206629
##########
src/main/java/org/apache/maven/shared/archiver/ManifestSection.java:
##########
@@ -31,17 +31,25 @@ public class ManifestSection {
private final Map<String, String> manifestEntries = new LinkedHashMap<>();
/**
- * <p>addManifestEntry.</p>
+ * Adds a single manifest entry.
*
- * @param key the key of the manifest entry
- * @param value the appropriate value
+ * <p>A {@code null} value is accepted and will produce an empty-string attribute in the manifest.
Review Comment:
Fixed in 9a1d474 — applied consistently across both ManifestSection and MavenArchiveConfiguration.
##########
src/main/java/org/apache/maven/shared/archiver/ManifestSection.java:
##########
@@ -31,17 +31,25 @@ public class ManifestSection {
private final Map<String, String> manifestEntries = new LinkedHashMap<>();
/**
- * <p>addManifestEntry.</p>
+ * Adds a single manifest entry.
*
- * @param key the key of the manifest entry
- * @param value the appropriate value
+ * <p>A {@code null} value is accepted and will produce an empty-string attribute in the manifest.
+ * This is intentional: it allows callers to explicitly request an empty manifest entry.
+ * Be careful not to pass {@code null} unintentionally (e.g., from an unguarded map lookup),
+ * as the resulting empty attribute will not indicate any error.</p>
+ *
+ * @param key the manifest attribute name
+ * @param value the manifest attribute value; {@code null} is treated as an empty string
*/
public void addManifestEntry(String key, String value) {
manifestEntries.put(key, value);
}
/**
- * <p>Getter for the field <code>manifestEntries</code>.</p>
+ * Returns the manifest entries map.
+ *
+ * <p>Values in the returned map may be {@code null}; such entries are treated as empty strings
Review Comment:
Fixed in 9a1d474.
##########
src/main/java/org/apache/maven/shared/archiver/ManifestSection.java:
##########
@@ -68,9 +76,14 @@ public void setName(String name) {
}
/**
- * <p>addManifestEntries.</p>
+ * Adds all entries from the given map as manifest attributes.
+ *
+ * <p>A {@code null} value in the map is accepted and will produce an empty-string attribute
Review Comment:
Fixed in 9a1d474.
##########
src/main/java/org/apache/maven/shared/archiver/ManifestSection.java:
##########
@@ -68,9 +76,14 @@ public void setName(String name) {
}
/**
- * <p>addManifestEntries.</p>
+ * Adds all entries from the given map as manifest attributes.
+ *
+ * <p>A {@code null} value in the map is accepted and will produce an empty-string attribute
+ * in the manifest. This is intentional: it allows callers to explicitly request an empty
+ * manifest entry. Be careful not to pass {@code null} values unintentionally, as they will
+ * not indicate any error.</p>
*
- * @param map the map to add
+ * @param map the manifest entries to add; map values may be {@code null}, which are treated as empty strings
Review Comment:
Fixed in 9a1d474.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] docs: document null-value behaviour in manifest entry methods [maven-archiver]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet merged PR #411:
URL: https://github.com/apache/maven-archiver/pull/411
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] docs: document null-value behaviour in manifest entry methods [maven-archiver]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #411:
URL: https://github.com/apache/maven-archiver/pull/411#discussion_r4108650087
##########
src/main/java/org/apache/maven/shared/archiver/MavenArchiveConfiguration.java:
##########
@@ -152,19 +152,29 @@ public void setManifest(ManifestConfiguration manifest) {
}
/**
- * <p>addManifestEntry.</p>
+ * Adds a single manifest entry.
*
- * @param key the key of the entry
- * @param value the value of the entry
+ * <p>A {@code null} value is accepted and will produce an empty-string attribute in the manifest.
+ * This is intentional: it allows callers to explicitly request an empty manifest entry.
+ * Be careful not to pass {@code null} unintentionally (e.g., from an unguarded map lookup),
+ * as the resulting empty attribute will not indicate any error.</p>
+ *
+ * @param key the manifest attribute name
+ * @param value the manifest attribute value; {@code null} is treated as an empty string
Review Comment:
Fixed in 5303e3083e47326e181bd2817aae05a5a71d52df: added the same null-value Javadoc to `ManifestSection.addManifestEntry()`, `addManifestEntries()`, and `getManifestEntries()`.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] docs: document null-value behaviour in manifest entry methods [maven-archiver]
Posted by "elharo (via GitHub)" <gi...@apache.org>.
elharo commented on code in PR #411:
URL: https://github.com/apache/maven-archiver/pull/411#discussion_r4144146495
##########
src/main/java/org/apache/maven/shared/archiver/ManifestSection.java:
##########
@@ -31,17 +31,25 @@ public class ManifestSection {
private final Map<String, String> manifestEntries = new LinkedHashMap<>();
/**
- * <p>addManifestEntry.</p>
+ * Adds a single manifest entry.
*
- * @param key the key of the manifest entry
- * @param value the appropriate value
+ * <p>A {@code null} value is accepted and will produce an empty-string attribute in the manifest.
Review Comment:
will produce --> produces
##########
src/main/java/org/apache/maven/shared/archiver/ManifestSection.java:
##########
@@ -31,17 +31,25 @@ public class ManifestSection {
private final Map<String, String> manifestEntries = new LinkedHashMap<>();
/**
- * <p>addManifestEntry.</p>
+ * Adds a single manifest entry.
*
- * @param key the key of the manifest entry
- * @param value the appropriate value
+ * <p>A {@code null} value is accepted and will produce an empty-string attribute in the manifest.
+ * This is intentional: it allows callers to explicitly request an empty manifest entry.
+ * Be careful not to pass {@code null} unintentionally (e.g., from an unguarded map lookup),
+ * as the resulting empty attribute will not indicate any error.</p>
+ *
+ * @param key the manifest attribute name
+ * @param value the manifest attribute value; {@code null} is treated as an empty string
*/
public void addManifestEntry(String key, String value) {
manifestEntries.put(key, value);
}
/**
- * <p>Getter for the field <code>manifestEntries</code>.</p>
+ * Returns the manifest entries map.
+ *
+ * <p>Values in the returned map may be {@code null}; such entries are treated as empty strings
Review Comment:
may be {@code null}. Such
##########
src/main/java/org/apache/maven/shared/archiver/ManifestSection.java:
##########
@@ -68,9 +76,14 @@ public void setName(String name) {
}
/**
- * <p>addManifestEntries.</p>
+ * Adds all entries from the given map as manifest attributes.
+ *
+ * <p>A {@code null} value in the map is accepted and will produce an empty-string attribute
Review Comment:
will produce --> produces
##########
src/main/java/org/apache/maven/shared/archiver/ManifestSection.java:
##########
@@ -68,9 +76,14 @@ public void setName(String name) {
}
/**
- * <p>addManifestEntries.</p>
+ * Adds all entries from the given map as manifest attributes.
+ *
+ * <p>A {@code null} value in the map is accepted and will produce an empty-string attribute
+ * in the manifest. This is intentional: it allows callers to explicitly request an empty
+ * manifest entry. Be careful not to pass {@code null} values unintentionally, as they will
+ * not indicate any error.</p>
*
- * @param map the map to add
+ * @param map the manifest entries to add; map values may be {@code null}, which are treated as empty strings
Review Comment:
entries to add. Map
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org