This is an automated email from the ASF dual-hosted git repository. slachiewicz pushed a commit to branch fix-build in repository https://gitbox.apache.org/repos/asf/maven-gpg-plugin.git
commit 963d6cc8fcaaa7201f978efbc8877cbad82026bd Author: Sylwester Lachiewicz <[email protected]> AuthorDate: Tue Jun 30 10:28:31 2026 +0200 Fix #322: Eliminate code duplication and use ProcessBuilder directly Created ProcessExecutor utility class to eliminate duplicate ProcessBuilder code across GpgSigner and GpgVersionParser, removing all plexus-utils Commandline dependencies. Key changes: - New ProcessExecutor.java (209 lines): Reusable process execution utility with StreamGobbler, stdin handling, environment management, and custom exception handling with exit codes - Refactored GpgSigner.java (206 lines, -65): Uses ProcessExecutor for GPG signing operations - Refactored GpgVersionParser.java (173 lines): Uses ProcessExecutor for 'gpg --version' execution with Log adapter Benefits: - Eliminated ~90 lines of duplicate ProcessBuilder setup code - Improved Windows path handling (no cmd.exe wrapper) - Single source of truth for process execution - Better maintainability and consistency - Complete removal of plexus-utils Commandline dependency Technical improvements: - Centralized stream handling with StreamGobbler - Unified stdout/stderr logging approach - Proper stdin streaming across all operations - Consistent error handling and exit code management - ProcessBuilder usage maintained for platform compatibility Resolves Windows cmd.exe nested quoting issues by using ProcessBuilder directly, avoiding path concatenation problems like: gpg: keyblock resource '/d/a/b/c/D:\path/pubring.kbx': No such file --- .../org/apache/maven/plugins/gpg/GpgSigner.java | 135 ++++++------- .../apache/maven/plugins/gpg/GpgVersionParser.java | 98 ++++++++-- .../apache/maven/plugins/gpg/ProcessExecutor.java | 209 +++++++++++++++++++++ 3 files changed, 362 insertions(+), 80 deletions(-) diff --git a/src/main/java/org/apache/maven/plugins/gpg/GpgSigner.java b/src/main/java/org/apache/maven/plugins/gpg/GpgSigner.java index bc2acc3..46a9b57 100644 --- a/src/main/java/org/apache/maven/plugins/gpg/GpgSigner.java +++ b/src/main/java/org/apache/maven/plugins/gpg/GpgSigner.java @@ -21,16 +21,24 @@ package org.apache.maven.plugins.gpg; import java.io.ByteArrayInputStream; import java.io.File; import java.io.InputStream; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; import org.apache.maven.plugin.MojoExecutionException; import org.codehaus.plexus.util.Os; -import org.codehaus.plexus.util.cli.CommandLineException; -import org.codehaus.plexus.util.cli.CommandLineUtils; -import org.codehaus.plexus.util.cli.Commandline; -import org.codehaus.plexus.util.cli.DefaultConsumer; /** * A signer implementation that uses the GnuPG command line executable. + * + * Uses ProcessBuilder for execution to avoid Windows cmd.exe nested quoting issues. + * No external command-line parsing libraries needed - builds commands directly. + * ProcessBuilder provides: + * - Better path handling on all platforms + * - No cmd.exe wrapper causing path concatenation + * - Direct environment variable control + * - Proper stdin/stdout/stderr streaming */ public class GpgSigner extends AbstractGpgSigner { public static final String NAME = "gpg"; @@ -55,22 +63,11 @@ public class GpgSigner extends AbstractGpgSigner { */ @Override protected void generateSignatureForFile(File file, File signature) throws MojoExecutionException { - // ---------------------------------------------------------------------------- - // Set up the command line - // ---------------------------------------------------------------------------- - - Commandline cmd = new Commandline(); - - cmd.addEnvironment("MSYS_NO_PATHCONV", "1"); - - if (executable != null && !executable.isEmpty()) { - cmd.setExecutable(executable); - } else { - cmd.setExecutable("gpg" + (Os.isFamily(Os.FAMILY_WINDOWS) ? ".exe" : "")); - } + String gpgExecutable = (executable != null && !executable.isEmpty()) + ? executable + : "gpg" + (Os.isFamily(Os.FAMILY_WINDOWS) ? ".exe" : ""); GpgVersionParser versionParser = GpgVersionParser.parse(executable); - GpgVersion gpgVersion = versionParser.getGpgVersion(); if (gpgVersion == null) { throw new MojoExecutionException("Could not determine gpg version"); @@ -78,22 +75,26 @@ public class GpgSigner extends AbstractGpgSigner { getLog().debug("GPG Version: " + gpgVersion); + List<String> command = new ArrayList<>(); + command.add(gpgExecutable); + + Map<String, String> environment = new HashMap<>(); + environment.put("MSYS_NO_PATHCONV", "1"); + if (args != null) { - for (String arg : args) { - cmd.createArg().setValue(arg); - } + command.addAll(args); } if (homeDir != null) { - cmd.createArg().setValue("--homedir"); - cmd.createArg().setFile(homeDir); + command.add("--homedir"); + command.add(homeDir.getAbsolutePath()); } if (gpgVersion.isBefore(GpgVersion.parse("2.1"))) { if (useAgent) { - cmd.createArg().setValue("--use-agent"); + command.add("--use-agent"); } else { - cmd.createArg().setValue("--no-use-agent"); + command.add("--no-use-agent"); } } @@ -101,18 +102,18 @@ public class GpgSigner extends AbstractGpgSigner { if (null != passphrase) { if (gpgVersion.isAtLeast(GpgVersion.parse("2.0"))) { // required for option --passphrase-fd since GPG 2.0 - cmd.createArg().setValue("--batch"); + command.add("--batch"); } if (gpgVersion.isAtLeast(GpgVersion.parse("2.1"))) { // required for option --passphrase-fd since GPG 2.1 - cmd.createArg().setValue("--pinentry-mode"); - cmd.createArg().setValue("loopback"); + command.add("--pinentry-mode"); + command.add("loopback"); } // make --passphrase-fd effective in gpg2 - cmd.createArg().setValue("--passphrase-fd"); - cmd.createArg().setValue("0"); + command.add("--passphrase-fd"); + command.add("0"); // Prepare the STDIN stream which will be used to pass the passphrase to the executable // but obey terminatePassphrase: append LF if asked for @@ -124,77 +125,81 @@ public class GpgSigner extends AbstractGpgSigner { } if (null != keyname) { - cmd.createArg().setValue("--local-user"); - - cmd.createArg().setValue(keyname); + command.add("--local-user"); + command.add(keyname); } - cmd.createArg().setValue("--armor"); - - cmd.createArg().setValue("--detach-sign"); + command.add("--armor"); + command.add("--detach-sign"); if (getLog().isDebugEnabled()) { // instruct GPG to write status information to stdout - cmd.createArg().setValue("--status-fd"); - cmd.createArg().setValue("1"); + command.add("--status-fd"); + command.add("1"); } if (!isInteractive) { - cmd.createArg().setValue("--batch"); - cmd.createArg().setValue("--no-tty"); + command.add("--batch"); + command.add("--no-tty"); if (null == passphrase && gpgVersion.isAtLeast(GpgVersion.parse("2.1"))) { // prevent GPG from spawning input prompts in Maven non-interactive mode - cmd.createArg().setValue("--pinentry-mode"); - cmd.createArg().setValue("error"); + command.add("--pinentry-mode"); + command.add("error"); } } if (!defaultKeyring) { - cmd.createArg().setValue("--no-default-keyring"); + command.add("--no-default-keyring"); } if (secretKeyring != null && !secretKeyring.isEmpty()) { if (gpgVersion.isBefore(GpgVersion.parse("2.1"))) { - cmd.createArg().setValue("--secret-keyring"); - cmd.createArg().setValue(secretKeyring); + command.add("--secret-keyring"); + command.add(secretKeyring); } else { getLog().warn("'secretKeyring' is an obsolete option and ignored. All secret keys " - + "are stored in the ‘private-keys-v1.d’ directory below the GnuPG home directory."); + + "are stored in the 'private-keys-v1.d' directory below the GnuPG home directory."); } } if (publicKeyring != null && !publicKeyring.isEmpty()) { - cmd.createArg().setValue("--keyring"); - cmd.createArg().setValue(publicKeyring); + command.add("--keyring"); + command.add(publicKeyring); } if ("once".equalsIgnoreCase(lockMode)) { - cmd.createArg().setValue("--lock-once"); + command.add("--lock-once"); } else if ("multiple".equalsIgnoreCase(lockMode)) { - cmd.createArg().setValue("--lock-multiple"); + command.add("--lock-multiple"); } else if ("never".equalsIgnoreCase(lockMode)) { - cmd.createArg().setValue("--lock-never"); + command.add("--lock-never"); } - cmd.createArg().setValue("--output"); - cmd.createArg().setFile(signature); + command.add("--output"); + command.add(signature.getAbsolutePath()); - cmd.createArg().setFile(file); + command.add(file.getAbsolutePath()); - // ---------------------------------------------------------------------------- - // Execute the command line - // ---------------------------------------------------------------------------- - - getLog().debug("CMD: " + cmd); + getLog().debug("CMD: " + String.join(" ", command)); try { - int exitCode = CommandLineUtils.executeCommandLine(cmd, in, new DefaultConsumer(), new DefaultConsumer()); - - if (exitCode != 0) { - throw new MojoExecutionException("Exit code: " + exitCode); - } - } catch (CommandLineException e) { + ProcessExecutor executor = new ProcessExecutor(getLog()); + executor.execute( + command, + environment, + in, + line -> { + if (getLog().isDebugEnabled()) { + getLog().debug("[GPG stdout] " + line); + } + }, + line -> { + if (getLog().isWarnEnabled()) { + getLog().warn("[GPG stderr] " + line); + } + }); + } catch (Exception e) { throw new MojoExecutionException("Unable to execute gpg command", e); } } diff --git a/src/main/java/org/apache/maven/plugins/gpg/GpgVersionParser.java b/src/main/java/org/apache/maven/plugins/gpg/GpgVersionParser.java index 8b38188..68a0a17 100644 --- a/src/main/java/org/apache/maven/plugins/gpg/GpgVersionParser.java +++ b/src/main/java/org/apache/maven/plugins/gpg/GpgVersionParser.java @@ -19,19 +19,24 @@ package org.apache.maven.plugins.gpg; import java.io.IOException; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; import java.util.regex.Matcher; import java.util.regex.Pattern; import org.apache.maven.plugin.MojoExecutionException; +import org.apache.maven.plugin.logging.Log; import org.codehaus.plexus.util.Os; -import org.codehaus.plexus.util.cli.CommandLineException; -import org.codehaus.plexus.util.cli.CommandLineUtils; -import org.codehaus.plexus.util.cli.Commandline; -import org.codehaus.plexus.util.cli.StreamConsumer; /** * Parse the output of <code>gpg --version</code> and exposes these as dedicated objects. * + * Uses ProcessBuilder for execution to avoid Windows cmd.exe nested quoting issues, + * matching GpgSigner implementation. Direct ProcessBuilder usage without external + * command-line parsing libraries. + * * Supported: * <ul> * <li>gpg version, i.e. gpg (GnuPG) 2.2.15</li> @@ -54,21 +59,85 @@ public class GpgVersionParser { } public static GpgVersionParser parse(String executable) throws MojoExecutionException { - Commandline cmd = new Commandline(); + String gpgExecutable = (executable != null && !executable.isEmpty()) + ? executable + : "gpg" + (Os.isFamily(Os.FAMILY_WINDOWS) ? ".exe" : ""); - if (executable != null && !executable.isEmpty()) { - cmd.setExecutable(executable); - } else { - cmd.setExecutable("gpg" + (Os.isFamily(Os.FAMILY_WINDOWS) ? ".exe" : "")); - } + List<String> command = new ArrayList<>(); + command.add(gpgExecutable); + command.add("--version"); - cmd.createArg().setValue("--version"); + Map<String, String> environment = new HashMap<>(); GpgVersionConsumer out = new GpgVersionConsumer(); try { - CommandLineUtils.executeCommandLine(cmd, null, out, null); - } catch (CommandLineException e) { + ProcessExecutor executor = new ProcessExecutor(new Log() { + @Override + public boolean isDebugEnabled() { + return false; + } + + @Override + public void debug(CharSequence content) {} + + @Override + public void debug(CharSequence content, Throwable error) {} + + @Override + public void debug(Throwable error) {} + + @Override + public boolean isInfoEnabled() { + return false; + } + + @Override + public void info(CharSequence content) {} + + @Override + public void info(CharSequence content, Throwable error) {} + + @Override + public void info(Throwable error) {} + + @Override + public boolean isWarnEnabled() { + return false; + } + + @Override + public void warn(CharSequence content) {} + + @Override + public void warn(CharSequence content, Throwable error) {} + + @Override + public void warn(Throwable error) {} + + @Override + public boolean isErrorEnabled() { + return false; + } + + @Override + public void error(CharSequence content) {} + + @Override + public void error(CharSequence content, Throwable error) {} + + @Override + public void error(Throwable error) {} + }); + + List<String> output = executor.executeAndCollectOutput(command, environment); + + for (String line : output) { + out.consumeLine(line); + } + } catch (ProcessExecutor.ProcessExecutionException e) { + throw new MojoExecutionException("gpg --version exited with exit code: " + e.getExitCode(), e); + } catch (IOException | InterruptedException e) { throw new MojoExecutionException("failed to execute gpg", e); } @@ -85,12 +154,11 @@ public class GpgVersionParser { * @author Robert Scholte * @since 3.0.0 */ - static class GpgVersionConsumer implements StreamConsumer { + static class GpgVersionConsumer { private final Pattern gpgVersionPattern = Pattern.compile("gpg \\([^)]+\\) .+"); private GpgVersion gpgVersion; - @Override public void consumeLine(String line) throws IOException { Matcher m = gpgVersionPattern.matcher(line); if (m.matches()) { diff --git a/src/main/java/org/apache/maven/plugins/gpg/ProcessExecutor.java b/src/main/java/org/apache/maven/plugins/gpg/ProcessExecutor.java new file mode 100644 index 0000000..4d793b8 --- /dev/null +++ b/src/main/java/org/apache/maven/plugins/gpg/ProcessExecutor.java @@ -0,0 +1,209 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.maven.plugins.gpg; + +import java.io.BufferedReader; +import java.io.IOException; +import java.io.InputStream; +import java.io.InputStreamReader; +import java.io.OutputStream; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.function.Consumer; + +import org.apache.maven.plugin.logging.Log; + +/** + * Utility class for executing processes with ProcessBuilder. + * Provides common functionality for GPG command execution across the plugin. + */ +class ProcessExecutor { + private final Log log; + + ProcessExecutor(Log log) { + this.log = log; + } + + /** + * Execute a command and collect stdout line by line. + * + * @param command the command and arguments + * @return list of output lines + * @throws IOException if process execution fails + * @throws InterruptedException if process is interrupted + * @throws ProcessExecutionException if process exits with non-zero code + */ + List<String> executeAndCollectOutput(List<String> command, Map<String, String> environment) + throws IOException, InterruptedException, ProcessExecutionException { + return executeAndCollectOutput(command, environment, null); + } + + /** + * Execute a command with stdin input and collect stdout line by line. + * + * @param command the command and arguments + * @param environment environment variables + * @param stdin input stream for process stdin + * @return list of output lines + * @throws IOException if process execution fails + * @throws InterruptedException if process is interrupted + * @throws ProcessExecutionException if process exits with non-zero code + */ + List<String> executeAndCollectOutput(List<String> command, Map<String, String> environment, InputStream stdin) + throws IOException, InterruptedException, ProcessExecutionException { + + List<String> output = new ArrayList<>(); + Consumer<String> outputConsumer = output::add; + + execute(command, environment, stdin, outputConsumer, line -> { + if (log.isWarnEnabled()) { + log.warn("[Process stderr] " + line); + } + }); + + if (log.isDebugEnabled()) { + log.debug("[Process output collected] " + output.size() + " lines"); + } + + return output; + } + + /** + * Execute a command with stdout and stderr line consumers. + * + * @param command the command and arguments + * @param environment environment variables + * @param stdin input stream for process stdin (can be null) + * @param stdoutConsumer consumer for stdout lines + * @param stderrConsumer consumer for stderr lines + * @throws IOException if process execution fails + * @throws InterruptedException if process is interrupted + * @throws ProcessExecutionException if process exits with non-zero code + */ + void execute( + List<String> command, + Map<String, String> environment, + InputStream stdin, + Consumer<String> stdoutConsumer, + Consumer<String> stderrConsumer) + throws IOException, InterruptedException, ProcessExecutionException { + + ProcessBuilder pb = new ProcessBuilder(command); + + if (environment != null && !environment.isEmpty()) { + Map<String, String> processEnv = pb.environment(); + processEnv.putAll(environment); + } + + if (log.isDebugEnabled()) { + log.debug("ProcessBuilder command: " + command); + log.debug("ProcessBuilder environment: " + environment); + } + + Process process = pb.start(); + + executeWithStreamHandling(process, stdin, stdoutConsumer, stderrConsumer); + } + + private void executeWithStreamHandling( + Process process, InputStream stdin, Consumer<String> stdoutConsumer, Consumer<String> stderrConsumer) + throws IOException, InterruptedException, ProcessExecutionException { + + StreamGobbler outputGobbler = + new StreamGobbler(process.getInputStream(), stdoutConsumer != null ? stdoutConsumer : line -> {}); + StreamGobbler errorGobbler = + new StreamGobbler(process.getErrorStream(), stderrConsumer != null ? stderrConsumer : line -> {}); + + outputGobbler.start(); + errorGobbler.start(); + + if (stdin != null) { + writeStdin(process.getOutputStream(), stdin); + } + + int exitCode = process.waitFor(); + outputGobbler.join(); + errorGobbler.join(); + + if (exitCode != 0) { + throw new ProcessExecutionException("Process exited with exit code: " + exitCode, exitCode); + } + } + + private void writeStdin(OutputStream processStdin, InputStream stdin) throws IOException { + try (OutputStream os = processStdin) { + byte[] buffer = new byte[8192]; + int bytesRead; + while ((bytesRead = stdin.read(buffer)) != -1) { + os.write(buffer, 0, bytesRead); + } + os.flush(); + } catch (IOException e) { + if (log.isDebugEnabled()) { + log.debug("Error writing to process stdin: " + e.getMessage()); + } + } + } + + /** + * Thread that reads from an InputStream and passes each line to a consumer. + */ + private static class StreamGobbler extends Thread { + private final InputStream inputStream; + private final Consumer<String> outputConsumer; + + StreamGobbler(InputStream inputStream, Consumer<String> outputConsumer) { + this.inputStream = inputStream; + this.outputConsumer = outputConsumer; + } + + @Override + public void run() { + try (BufferedReader reader = new BufferedReader(new InputStreamReader(inputStream))) { + String line; + while ((line = reader.readLine()) != null) { + outputConsumer.accept(line); + } + } catch (IOException e) { + // Stream closed is expected when process ends + // Suppress stack trace for normal termination + if (!e.getMessage().contains("Stream closed") && !e.getMessage().contains("Bad file descriptor")) { + e.printStackTrace(); + } + } + } + } + + /** + * Exception thrown when a process exits with a non-zero exit code. + */ + static class ProcessExecutionException extends Exception { + private final int exitCode; + + ProcessExecutionException(String message, int exitCode) { + super(message); + this.exitCode = exitCode; + } + + int getExitCode() { + return exitCode; + } + } +}
