gnodet-bot commented on code in PR #13589:
URL: https://github.com/apache/maven/pull/13589#discussion_r4301819637
##########
maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java:
##########
@@ -1013,6 +1016,34 @@ private List<File> parseExtClasspath(CliRequest
cliRequest) {
return jars;
}
+ /**
+ * Reads a password from the console and, when there is none because the
streams are redirected, from the first
+ * line of the given input.
+ *
+ * @throws IllegalStateException if no password could be read, so that an
empty one is not encrypted
+ */
+ static String readPassword(Console console, InputStream in, String prompt)
throws IOException {
+ String password;
+ if (console != null) {
+ char[] chars = console.readPassword(prompt);
+ password = (chars == null) ? null : String.copyValueOf(chars);
+
+ if (chars != null) {
+ // Sun/Oracle advises to empty the char array
+ java.util.Arrays.fill(chars, ' ');
+ }
+ } else {
+ // do not close the reader, that would close the standard input
+ password = new BufferedReader(new InputStreamReader(in,
Charset.defaultCharset())).readLine();
Review Comment:
⚠️ **`prompt` is silently ignored in the `else` branch.**
When there is no console, the method reads from `in` but never displays the
prompt to the user. This is a silent regression compared to the old code, which
at least called `cons.readPassword("Password: ")` when the console was present.
In the fallback path the prompt string is simply dropped — the user gets no
indication that they should type a password.
If there's no portable way to print to the terminal in this context (since
`System.console()` is null), at minimum the Javadoc should document that
`prompt` is unused when `console` is `null`.
##########
maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java:
##########
@@ -1013,6 +1016,34 @@ private List<File> parseExtClasspath(CliRequest
cliRequest) {
return jars;
}
+ /**
+ * Reads a password from the console and, when there is none because the
streams are redirected, from the first
+ * line of the given input.
+ *
+ * @throws IllegalStateException if no password could be read, so that an
empty one is not encrypted
+ */
+ static String readPassword(Console console, InputStream in, String prompt)
throws IOException {
Review Comment:
📝 **Javadoc is incomplete.**
- `@throws IOException` is declared in the method signature but not
documented.
- `@param` tags are missing for all three parameters (`console`, `in`,
`prompt`).
- `@return` tag is missing.
```suggestion
/**
* Reads a password from the console and, when there is none because the
streams are redirected, from the first
* line of the given input.
*
* @param console the system console, or {@code null} when streams are
redirected
* @param in the input stream to fall back to when {@code console} is
{@code null}
* @param prompt the prompt to display when reading from the console
(ignored when {@code console} is {@code null})
* @return the password read from the console or input stream
* @throws IOException if an I/O error occurs while reading from the
input stream
* @throws IllegalStateException if no password could be read, so that
an empty one is not encrypted
*/
static String readPassword(Console console, InputStream in, String
prompt) throws IOException {
```
--
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]
