arturobernalg opened a new pull request, #179:
URL: https://github.com/apache/maven-jarsigner-plugin/pull/179
This addresses three related issues in the same small area of the plugin:
#146, #150 and #153.
`decrypt()` dereferenced `securityDispatcher` without a null check and
always called it, even when the value being decrypted (storepass or keypass)
was null -- both `processArchive()` and `createRequest()` call `decrypt()`
unconditionally, so every build that leaves storepass/keypass unset was already
calling `securityDispatcher.decrypt(null)` (#150).
The no-arg constructors on `JarsignerSignMojo` and `JarsignerVerifyMojo`
("for testing; invoked via reflection") pass `null` for `securityDispatcher` to
the superclass. Today the test harness (`MojoTestCreator`) always sets a real
dispatcher back in via reflection, so this isn't reachable in the current test
suite, but any test or future code path that instantiates a Mojo this way and
calls `execute()` without restoring the field crashes with a bare
`NullPointerException` instead of a message that says what went wrong (#146).
`decrypt()` now returns `null` immediately when the input is `null` (so a
missing `securityDispatcher` is no longer reachable through the default,
no-password configuration), and separately guards against `securityDispatcher`
being `null`, raising a `MojoExecutionException` instead of an NPE.
For #153, I agree the original ask (include more context, like the password
value, in the error) isn't something to act on directly -- that would mean
logging password-adjacent content. What the error message is actually missing
is *which* configuration value failed: jarsigner has two independent passwords
(storepass, keypass), and a decrypt failure for one looked identical to a
failure for the other. Added a `fieldName` parameter to `decrypt()` so both the
log message and the exception name the field (`storepass` or `keypass`). The
existing one-arg `decrypt(String)` overload is kept for source compatibility
and defaults to a generic `"password"` label.
Added two tests to `JarsignerSignMojoTest`: one confirming `decrypt()` never
touches the `SecDispatcher` when no password is configured, one confirming a
missing `SecDispatcher` fails with a `MojoExecutionException` naming the field
(`storepass`) rather than an NPE. Confirmed both fail on the previous code (an
unwanted dispatcher interaction, and a message lacking `storepass`) and pass
with the fix. Full module test suite (54 tests), checkstyle and spotless all
still pass.
- [x] I hereby declare this contribution to be licensed under the Apache
License Version 2.0, January 2004
- [x] I hereby declare that I have filed an Apache Individual Contributor
License Agreement.
--
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]
