cstamas opened a new issue, #16:
URL: https://github.com/apache/maven-executor/issues/16
### Affected version
1.0.0
### Bug description
Bug Report: maven-executor
==========================
HIGH SEVERITY
-------------
1. toBuilder().argument(String) throws UnsupportedOperationException
ExecutorRequest.java:150-165, 249-255
toBuilder() passes arguments() (an unmodifiable list from Impl)
into the Builder constructor,
which stores the reference directly. When argument(String) is
called, it invokes
this.arguments.add(...) on the unmodifiable list, crashing at runtime.
2. MAVEN_ARGS leaks to child process in ForkedMavenExecutor
ForkedMavenExecutor.java:109, 143, 149-153
Line 109 reads System.getenv("MAVEN_ARGS") from the parent process.
Line 143 does
env.remove("MAVEN_ARGS"), but env is a local HashMap containing
only request-specified
variables, not the parent process environment. Line 152 calls
pb.environment().putAll(env)
but MAVEN_ARGS from the parent is never removed from the inherited
environment, so the
child process also inherits it.
3. extractMTPSingleArgument throws StringIndexOutOfBoundsException
MavenToolProvider.java (java9):146
When an argument like -MEX-mavenHome is passed without = and a value,
argument.substring(key.length() + 1) throws
StringIndexOutOfBoundsException.
4. Null values from extractMTPMapArgument produce string "null" in output
MavenToolProvider.java (java9):165-169 ->
ForkedMavenExecutor.java:127 -> DockerExeExecutor.java:91
When -MEX-env=MY_VAR is passed without a =value, the map stores
null. This propagates to
"MY_VAR=null" in DockerExeExecutor and "-Dkey=null" in
ForkedMavenExecutor.
5. ToolboxExecutorTool.doExecute() uses request.stdOut() (an
OutputStream reference) instead
of result.stdOutString() in error messages
ToolboxExecutorTool.java:144-146
The error message prints request.stdOut().orElse(null) which
returns the OutputStream object
(e.g., java.io.ByteArrayOutputStream@123456), not the captured
output. The actual stdout/stderr
content is in result.stdOutString()/result.stdErrString() but is never
read.
MEDIUM SEVERITY
---------------
6. stdIn InputStream is never closed (resource leak)
ProcessBuilderExecutorSupport.java:149-158
In the stdinPump thread, the stdIn InputStream is read from but
never closed. The
try-with-resources only closes in (the process's OutputStream). The
Javadoc says
"The stream is closed once tool execution is finished" but this
does not happen.
7. fs4WithDollar test uses MAVEN3_HOME instead of MAVEN4_HOME (copy-paste
error)
MavenExecutorTestSupport.java:329
The @Disabled test fs4WithDollar (intended for Maven 4) references
Environment.MAVEN3_HOME
at line 329.
8. System.setProperties(null) creates thread-safety window
EmbeddedMavenExecutor.java:383
prepareProperties() calls System.setProperties(null) followed by
properties.putAll(System.getProperties()). Between these two calls,
any concurrent thread
accessing System.getProperties() may get null or empty properties.
9. ExecutorHelperImpl.close() does not catch RuntimeException from
executor close()
ExecutorHelperImpl.java:88-109
Only ExecutorException is caught when closing executors. If an
executor's close() throws
a RuntimeException (e.g., NullPointerException), it propagates
immediately and prevents
subsequent executors from being closed.
10. UncheckedIOException not wrapped as ExecutorException in
TestContainersExecutor
TestContainersExecutor.java:161-166
detectUid() wraps IOException in UncheckedIOException (a
RuntimeException). The execute()
method's catch blocks only catch IOException. The
UncheckedIOException propagates uncaught.
11. NPE if resource files are missing in TestProjects.createSimpleProject
TestProjects.java:37, 40
TestProjects.class.getResourceAsStream(...) can return null, and
Files.copy(InputStream, Path)
will throw NullPointerException instead of a clear "resource not
found" message.
LOW SEVERITY
------------
12. extractMTPSingleArgument removes ALL matching arguments, not just the
first
MavenToolProvider.java (java9):145
allArguments.removeIf(s -> s.equals(argument)) removes every
occurrence, silently
discarding duplicates.
13. Builder.stdOut()/stdErr() don't clear the other stream
ExecutorRequest.java:324-334
Setting stdOut(...) sets grabOutputAsString = false but does not
clear this.stdErr,
and vice versa.
14. Temp directory deletion may fail in ForkedMavenExecutor.mavenVersion()
ForkedMavenExecutor.java:74, 93
Files.createTempDirectory(...) is deleted in finally via
Files.deleteIfExists(cwd), but
if the Maven process creates files inside this directory, the
delete fails with
DirectoryNotEmptyException.
15. nullInputStream.read(byte[], int, int) violates InputStream contract
IOTools.java:83-89
The read(byte[], int, int) contract requires throwing
IndexOutOfBoundsException if off is
negative, len is negative, or off + len > b.length. No bounds
validation is performed.
16. Deprecated ToolboxExecutorTool(Executor) hardcodes 0.15.8 vs current
0.15.14
ToolboxExecutorTool.java:53-56
The deprecated constructor uses an old cemented version while the
POM defines
version.toolbox as 0.15.14.
17. @DisabledOnOs comment vs annotation mismatch in Docker provider tests
TestContainersExecutorTest.java:38, DockerExeExecutorTest.java:38
Comment says "not supported on Windows" but annotation also
disables on OS.MAC.
--
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]
