Copilot commented on code in PR #398:
URL: https://github.com/apache/maven-wrapper/pull/398#discussion_r3124432604
##########
src/site/markdown/index.md:
##########
@@ -192,10 +192,10 @@ used.
To avoid supply-chain-attacks by downloading a corrupted artifact, it
is possible to specify checksums for both the *maven-wrapper.jar* and
the downloaded distribution. To apply verification, add the expected
-file's SHA-256 sum in hex notation, using only small caps, to
+file's SHA-256 or SHA-512 sum in hex notation, using only small caps, to
`maven-wrapper.properties`. The property for validating the
-*maven-wrapper.jar* file is named `wrapperSha256Sum` whereas the
-distribution file property is named `distributionSha256Sum`.
+*maven-wrapper.jar* file are named `wrapperSha256Sum` and `wrapperSha512Sum`
whereas the
+distribution file property are named `distributionSha256Sum` and
`distributionSha512Sum`.
Review Comment:
Grammar/wording issue: the new text uses singular/plural inconsistently
(e.g., "file ... are named" / "property are named"). Please rephrase so
subject/verb agreement is correct and it’s clear these are *properties*
(plural) for wrapper vs distribution checksums.
##########
maven-wrapper/src/main/java/org/apache/maven/wrapper/Installer.java:
##########
@@ -67,6 +67,8 @@ public Path createDist(WrapperConfiguration configuration)
throws Exception {
boolean alwaysUnpack = configuration.isAlwaysUnpack();
boolean verifyDistributionSha256Sum =
!configuration.getDistributionSha256Sum().isEmpty();
+ boolean verifyDistributionSha512Sum =
+ !configuration.getDistributionSha512Sum().isEmpty();
Review Comment:
`createDist` now calls `configuration.getDistributionSha512Sum().isEmpty()`.
Since `distributionSha512Sum` defaults to null in `WrapperConfiguration`, this
can throw a NullPointerException for callers that construct
`WrapperConfiguration` programmatically (backward-incompatible). Make this
check null-safe (e.g., treat null as "not set") or initialize
`distributionSha512Sum` to an empty string by default.
##########
maven-wrapper-distribution/src/resources/mvnw:
##########
@@ -310,6 +310,38 @@ if [ -n "$wrapperSha256Sum" ]; then
fi
fi
+# If specified, validate the SHA-512 sum of the Maven wrapper jar file
+wrapperSha512Sum=""
+while IFS="=" read -r key value; do
+ case "$key" in wrapperSha512Sum)
+ wrapperSha512Sum=$(trim "${value-}")
+ break
+ ;;
+ esac
+done <"$MAVEN_PROJECTBASEDIR/.mvn/wrapper/maven-wrapper.properties"
+if [ -n "$wrapperSha512Sum" ]; then
+ wrapperSha512Result=false
+ if command -v sha512sum >/dev/null; then
+ if echo "$wrapperSha512Sum $wrapperJarPath" | sha512sum -c - >/dev/null
2>&1; then
+ wrapperSha512Result=true
+ fi
+ elif command -v shasum >/dev/null; then
+ if echo "$wrapperSha512Sum $wrapperJarPath" | shasum -a 512 -c >/dev/null
2>&1; then
+ wrapperSha512Result=true
Review Comment:
The new wrapper SHA-512 verification uses `sha512sum -c -` without
`--strict`, while the SHA-256 verification uses `sha256sum -c --strict -`.
Using `--strict` here too would make checksum validation behavior consistent
and avoid ignoring malformed input lines.
##########
maven-wrapper-distribution/src/resources/only-mvnw:
##########
@@ -249,6 +250,33 @@ if [ -n "${distributionSha256Sum-}" ]; then
fi
fi
+# If specified, validate the SHA-512 sum of the Maven distribution zip file
+if [ -n "${distributionSha512Sum-}" ]; then
+ distributionSha512Result=false
+ if [ "$MVN_CMD" = mvnd.sh ]; then
+ echo "Checksum validation is not supported for maven-mvnd." >&2
+ echo "Please disable validation by removing 'distributionSha512Sum' from
your maven-wrapper.properties." >&2
+ exit 1
+ elif command -v sha512sum >/dev/null; then
+ if echo "$distributionSha512Sum $TMP_DOWNLOAD_DIR/$distributionUrlName" |
sha512sum -c - >/dev/null 2>&1; then
+ distributionSha512Result=true
+ fi
+ elif command -v shasum >/dev/null; then
+ if echo "$distributionSha512Sum $TMP_DOWNLOAD_DIR/$distributionUrlName" |
shasum -a 512 -c >/dev/null 2>&1; then
+ distributionSha512Result=true
Review Comment:
The SHA-512 verification path uses `sha512sum -c -` without `--strict`,
while the SHA-256 branch uses `sha256sum -c --strict -`. For consistency and to
avoid accepting malformed checksum lines, consider adding `--strict` to the
SHA-512 check as well.
--
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]
