From 8bb0c2755a23154b74b548a987d103081966c5eb Mon Sep 17 00:00:00 2001 From: Elliotte Rusty Harold Date: Wed, 29 Jul 2026 14:00:49 +0000 Subject: [PATCH 1/2] fix #12583: Inverted file existence check in DefaultTransport.put() --- .../apache/maven/impl/DefaultTransport.java | 2 +- .../maven/impl/DefaultTransportTest.java | 62 +++++++++++++++++++ 2 files changed, 63 insertions(+), 1 deletion(-) create mode 100644 impl/maven-impl/src/test/java/org/apache/maven/impl/DefaultTransportTest.java diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultTransport.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultTransport.java index f93e5f8d5d66..68490fa40cd4 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultTransport.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/DefaultTransport.java @@ -98,7 +98,7 @@ public Optional getString(URI relativeSource, Charset charset) { public void put(Path source, URI relativeTarget) { requireNonNull(source, "source is null"); requireNonNull(relativeTarget, "relativeTarget is null"); - if (Files.isRegularFile(source)) { + if (!Files.isRegularFile(source)) { throw new IllegalArgumentException("source file does not exist or is not a file"); } if (relativeTarget.isAbsolute()) { diff --git a/impl/maven-impl/src/test/java/org/apache/maven/impl/DefaultTransportTest.java b/impl/maven-impl/src/test/java/org/apache/maven/impl/DefaultTransportTest.java new file mode 100644 index 000000000000..bb37f5b598c7 --- /dev/null +++ b/impl/maven-impl/src/test/java/org/apache/maven/impl/DefaultTransportTest.java @@ -0,0 +1,62 @@ +/* + * 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.impl; + +import java.io.IOException; +import java.net.URI; +import java.nio.file.Files; +import java.nio.file.Path; + +import org.eclipse.aether.spi.connector.transport.Transporter; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.Mockito.mock; + +class DefaultTransportTest { + + @Test + void testPutWithNonExistentFileThrows() { + Transporter transporter = mock(Transporter.class); + DefaultTransport transport = new DefaultTransport(URI.create("http://example.com/test/"), transporter); + Path nonExistentFile = Path.of("/nonexistent/file.txt"); + assertThrows(IllegalArgumentException.class, () -> transport.put(nonExistentFile, URI.create("dest.txt"))); + } + + @Test + void testPutWithExistingFileSucceeds(@TempDir Path tempDir) throws IOException { + Path sourceFile = tempDir.resolve("source.txt"); + Files.writeString(sourceFile, "test content"); + + Transporter transporter = mock(Transporter.class); + DefaultTransport transport = new DefaultTransport(URI.create("http://example.com/test/"), transporter); + URI dest = URI.create("dest.txt"); + assertDoesNotThrow(() -> transport.put(sourceFile, dest)); + } + + @Test + void testPutBytesSucceeds(@TempDir Path tempDir) { + Transporter transporter = mock(Transporter.class); + DefaultTransport transport = new DefaultTransport(URI.create("http://example.com/test/"), transporter); + URI dest = URI.create("dest.txt"); + assertDoesNotThrow(() -> transport.putBytes("test content".getBytes(), dest)); + } +} From c67af90f7ebc516893c5a20500b1cf3d6de93e13 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Wed, 29 Jul 2026 17:57:48 +0200 Subject: [PATCH 2/2] Address review comments: improve test robustness - Use @TempDir for non-existent file test instead of hardcoded path - Remove unused @TempDir parameter from testPutBytesSucceeds - Replace assertDoesNotThrow with verify(transporter).put() to confirm delegation chain actually invokes the transporter Co-Authored-By: Claude Opus 4.6 --- .../maven/impl/DefaultTransportTest.java | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/impl/maven-impl/src/test/java/org/apache/maven/impl/DefaultTransportTest.java b/impl/maven-impl/src/test/java/org/apache/maven/impl/DefaultTransportTest.java index bb37f5b598c7..1e96072a79a2 100644 --- a/impl/maven-impl/src/test/java/org/apache/maven/impl/DefaultTransportTest.java +++ b/impl/maven-impl/src/test/java/org/apache/maven/impl/DefaultTransportTest.java @@ -18,45 +18,48 @@ */ package org.apache.maven.impl; -import java.io.IOException; import java.net.URI; import java.nio.file.Files; import java.nio.file.Path; +import org.eclipse.aether.spi.connector.transport.PutTask; import org.eclipse.aether.spi.connector.transport.Transporter; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; -import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; class DefaultTransportTest { @Test - void testPutWithNonExistentFileThrows() { + void testPutWithNonExistentFileThrows(@TempDir Path tempDir) { Transporter transporter = mock(Transporter.class); DefaultTransport transport = new DefaultTransport(URI.create("http://example.com/test/"), transporter); - Path nonExistentFile = Path.of("/nonexistent/file.txt"); + Path nonExistentFile = tempDir.resolve("missing.txt"); assertThrows(IllegalArgumentException.class, () -> transport.put(nonExistentFile, URI.create("dest.txt"))); } @Test - void testPutWithExistingFileSucceeds(@TempDir Path tempDir) throws IOException { + void testPutWithExistingFileSucceeds(@TempDir Path tempDir) throws Exception { Path sourceFile = tempDir.resolve("source.txt"); Files.writeString(sourceFile, "test content"); Transporter transporter = mock(Transporter.class); DefaultTransport transport = new DefaultTransport(URI.create("http://example.com/test/"), transporter); URI dest = URI.create("dest.txt"); - assertDoesNotThrow(() -> transport.put(sourceFile, dest)); + transport.put(sourceFile, dest); + verify(transporter).put(any(PutTask.class)); } @Test - void testPutBytesSucceeds(@TempDir Path tempDir) { + void testPutBytesSucceeds() throws Exception { Transporter transporter = mock(Transporter.class); DefaultTransport transport = new DefaultTransport(URI.create("http://example.com/test/"), transporter); URI dest = URI.create("dest.txt"); - assertDoesNotThrow(() -> transport.putBytes("test content".getBytes(), dest)); + transport.putBytes("test content".getBytes(), dest); + verify(transporter).put(any(PutTask.class)); } }