From 3f4dae73e67d329da3b70811d2103394ec0dde9a Mon Sep 17 00:00:00 2001 From: "Piotr P. Karwasz" Date: Mon, 13 Oct 2025 15:57:57 +0200 Subject: [PATCH 1/7] Fixes issues in `CloseShieldChannel` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two bugs in the `CloseShieldChannel` helper make it unreliable in practice: 1. **Type-erasure bug in `T wrap(T)`** The method signature only works correctly when `T` is an **interface** extending `Channel`. Since Java’s type system doesn’t allow constraining `T` to “interface types only,” this could lead to unexpected runtime `ClassCastException`s even though the code compiles successfully. 2. **Incomplete interface discovery** The implementation only inspected interfaces **directly** implemented by the given channel’s class, ignoring those inherited from its superclasses. As a result, proxies for types such as `FileChannel` did not expose any of the interfaces declared on `FileChannel` itself. #### Fixes This PR addresses both issues: * **Reworks the API signature** * Replaces `T wrap(T)` with its erasure: `Channel wrap(Channel)`. * Introduces a new overload: `T wrap(T, Class)`, which allows callers to explicitly specify the interface type they expect. This version fails fast with a clear `IllegalArgumentException` if the provided type is not an interface, instead of allowing a `ClassCastException` later. * **Improves interface collection logic** * Updates the implementation to include interfaces declared on superclasses, ensuring all relevant `Channel` interfaces are correctly proxied. --- .../io/channels/CloseShieldChannel.java | 37 +++++++++-- .../io/channels/CloseShieldChannelTest.java | 63 ++++++++++++++++--- 2 files changed, 86 insertions(+), 14 deletions(-) diff --git a/src/main/java/org/apache/commons/io/channels/CloseShieldChannel.java b/src/main/java/org/apache/commons/io/channels/CloseShieldChannel.java index a9a462da73b..7be297e20bb 100644 --- a/src/main/java/org/apache/commons/io/channels/CloseShieldChannel.java +++ b/src/main/java/org/apache/commons/io/channels/CloseShieldChannel.java @@ -40,11 +40,15 @@ public final class CloseShieldChannel { private static final Class[] EMPTY = {}; private static Set> collectChannelInterfaces(final Class type, final Set> out) { + Class currentType = type; // Visit interfaces - for (final Class iface : type.getInterfaces()) { - if (Channel.class.isAssignableFrom(iface) && out.add(iface)) { - collectChannelInterfaces(iface, out); + while (currentType != null) { + for (final Class iface : currentType.getInterfaces()) { + if (Channel.class.isAssignableFrom(iface) && out.add(iface)) { + collectChannelInterfaces(iface, out); + } } + currentType = currentType.getSuperclass(); } return out; } @@ -53,11 +57,10 @@ private static Set> collectChannelInterfaces(final Class type, final * Wraps a channel to shield it from being closed. * * @param channel The underlying channel to shield, not {@code null}. - * @param Any Channel type (interface or class). * @return A proxy that shields {@code close()} and enforces closed semantics on other calls. */ @SuppressWarnings({ "unchecked", "resource" }) // caller closes - public static T wrap(final T channel) { + public static Channel wrap(final Channel channel) { Objects.requireNonNull(channel, "channel"); // Fast path: already our shield if (Proxy.isProxyClass(channel.getClass()) && Proxy.getInvocationHandler(channel) instanceof CloseShieldChannelHandler) { @@ -66,10 +69,32 @@ public static T wrap(final T channel) { // Collect only Channel sub-interfaces. final Set> set = collectChannelInterfaces(channel.getClass(), new LinkedHashSet<>()); // fallback to root surface - return (T) Proxy.newProxyInstance(channel.getClass().getClassLoader(), // use delegate's loader + return (Channel) Proxy.newProxyInstance(channel.getClass().getClassLoader(), // use delegate's loader set.isEmpty() ? new Class[] { Channel.class } : set.toArray(EMPTY), new CloseShieldChannelHandler(channel)); } + /** + * Wraps a channel to shield it from being closed. + * + * @param channel The underlying channel to shield, not {@code null} and must implement {@code type}. + * @param type The interface the returned proxy must implement; + * the proxy will also implement all other {@link Channel} sub-interfaces that the delegate implements. + * @param A type that extends {@link Channel}. + * @return A proxy that shields {@code close()} and enforces closed semantics on other calls. + * @throws IllegalArgumentException if {@code type} is not an interface or if {@code channel} does not implement {@code type}. + */ + @SuppressWarnings({ "unchecked", "resource" }) // caller closes + public static T wrap(final T channel, Class type) { + Objects.requireNonNull(type, "type"); + if (!type.isInterface()) { + throw new IllegalArgumentException(type.getName() + " is not an interface"); + } + if (!type.isInstance(channel)) { + throw new IllegalArgumentException("channel of type " + channel.getClass().getName() + " is not an instance of " + type.getName()); + } + return type.cast(wrap(channel)); + } + private CloseShieldChannel() { // no instance } diff --git a/src/test/java/org/apache/commons/io/channels/CloseShieldChannelTest.java b/src/test/java/org/apache/commons/io/channels/CloseShieldChannelTest.java index ac7e85b7a89..6e15ba02bff 100644 --- a/src/test/java/org/apache/commons/io/channels/CloseShieldChannelTest.java +++ b/src/test/java/org/apache/commons/io/channels/CloseShieldChannelTest.java @@ -20,6 +20,7 @@ import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertNotSame; import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -32,11 +33,13 @@ import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; +import java.io.IOException; import java.nio.channels.AsynchronousByteChannel; import java.nio.channels.AsynchronousChannel; import java.nio.channels.ByteChannel; import java.nio.channels.Channel; import java.nio.channels.ClosedChannelException; +import java.nio.channels.FileChannel; import java.nio.channels.GatheringByteChannel; import java.nio.channels.InterruptibleChannel; import java.nio.channels.MulticastChannel; @@ -45,9 +48,12 @@ import java.nio.channels.ScatteringByteChannel; import java.nio.channels.SeekableByteChannel; import java.nio.channels.WritableByteChannel; +import java.nio.file.Path; import java.util.stream.Stream; +import org.apache.commons.io.FileUtils; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.MethodSource; @@ -116,8 +122,8 @@ void testCloseIsShielded(final Class channelClass) throws Exc @Test void testDoesNotDoubleWrap() { final ByteChannel channel = mock(ByteChannel.class); - final ByteChannel shield1 = CloseShieldChannel.wrap(channel); - final ByteChannel shield2 = CloseShieldChannel.wrap(shield1); + final ByteChannel shield1 = CloseShieldChannel.wrap(channel, ByteChannel.class); + final ByteChannel shield2 = CloseShieldChannel.wrap(shield1, ByteChannel.class); assertSame(shield1, shield2); } @@ -137,7 +143,7 @@ void testEquals(final Class channelClass) throws Exception { void testGatheringByteChannelMethods() throws Exception { final GatheringByteChannel channel = mock(GatheringByteChannel.class); when(channel.isOpen()).thenReturn(true); - final GatheringByteChannel shield = CloseShieldChannel.wrap(channel); + final GatheringByteChannel shield = CloseShieldChannel.wrap(channel, GatheringByteChannel.class); // Before close write() should delegate when(channel.write(null, 0, 0)).thenReturn(42L); assertEquals(42, shield.write(null, 0, 0)); @@ -162,7 +168,7 @@ void testHashCode(final Class channelClass) throws Exception void testNetworkChannelMethods() throws Exception { final NetworkChannel channel = mock(NetworkChannel.class); when(channel.isOpen()).thenReturn(true); - final NetworkChannel shield = CloseShieldChannel.wrap(channel); + final NetworkChannel shield = CloseShieldChannel.wrap(channel, NetworkChannel.class); // Before close getOption(), setOption(), getLocalAddress() and bind() should delegate when(channel.getOption(null)).thenReturn("foo"); when(channel.setOption(null, null)).thenReturn(channel); @@ -201,7 +207,7 @@ void testPreservesInterfaces(final Class channelClass) { void testReadableByteChannelMethods() throws Exception { final ReadableByteChannel channel = mock(ReadableByteChannel.class); when(channel.isOpen()).thenReturn(true); - final ReadableByteChannel shield = CloseShieldChannel.wrap(channel); + final ReadableByteChannel shield = CloseShieldChannel.wrap(channel, ReadableByteChannel.class); // Before close read() should delegate when(channel.read(null)).thenReturn(42); assertEquals(42, shield.read(null)); @@ -216,7 +222,7 @@ void testReadableByteChannelMethods() throws Exception { void testScatteringByteChannelMethods() throws Exception { final ScatteringByteChannel channel = mock(ScatteringByteChannel.class); when(channel.isOpen()).thenReturn(true); - final ScatteringByteChannel shield = CloseShieldChannel.wrap(channel); + final ScatteringByteChannel shield = CloseShieldChannel.wrap(channel, ScatteringByteChannel.class); // Before close read() should delegate when(channel.read(null, 0, 0)).thenReturn(42L); assertEquals(42, shield.read(null, 0, 0)); @@ -231,7 +237,7 @@ void testScatteringByteChannelMethods() throws Exception { void testSeekableByteChannelMethods() throws Exception { final SeekableByteChannel channel = mock(SeekableByteChannel.class); when(channel.isOpen()).thenReturn(true); - final SeekableByteChannel shield = CloseShieldChannel.wrap(channel); + final SeekableByteChannel shield = CloseShieldChannel.wrap(channel, SeekableByteChannel.class); // Before close position() and size() should delegate when(channel.position()).thenReturn(42L); when(channel.size()).thenReturn(84L); @@ -270,7 +276,7 @@ void testToString(final Class channelClass) throws Exception void testWritableByteChannelMethods() throws Exception { final WritableByteChannel channel = mock(WritableByteChannel.class); when(channel.isOpen()).thenReturn(true); - final WritableByteChannel shield = CloseShieldChannel.wrap(channel); + final WritableByteChannel shield = CloseShieldChannel.wrap(channel, WritableByteChannel.class); // Before close write() should delegate when(channel.write(null)).thenReturn(42); assertEquals(42, shield.write(null)); @@ -280,4 +286,45 @@ void testWritableByteChannelMethods() throws Exception { assertThrows(ClosedChannelException.class, () -> shield.write(null)); verifyNoMoreInteractions(channel); } + + @Test + void testCorrectlyDetectsInterfaces(@TempDir Path tempDir) throws IOException { + final Path testFile = tempDir.resolve("test.txt"); + FileUtils.touch(testFile.toFile()); + try (FileChannel channel = FileChannel.open(testFile); Channel shield = CloseShieldChannel.wrap(channel)) { + assertInstanceOf(SeekableByteChannel.class, shield); + assertInstanceOf(GatheringByteChannel.class, shield); + assertInstanceOf(WritableByteChannel.class, shield); + assertInstanceOf(ScatteringByteChannel.class, shield); + assertInstanceOf(ReadableByteChannel.class, shield); + assertInstanceOf(InterruptibleChannel.class, shield); + assertInstanceOf(ByteChannel.class, shield); + assertInstanceOf(Channel.class, shield); + // These are not interfaces, so can not be implemented + assertFalse(shield instanceof FileChannel, "not FileChannel"); + } + } + + @Test + void testThrowsIllegalArgumentException(@TempDir Path tempDir) throws Exception { + final Path testFile = tempDir.resolve("test.txt"); + FileUtils.touch(testFile.toFile()); + // Argument is not an interface + try (FileChannel channel = FileChannel.open(testFile)) { + final IllegalArgumentException ex = assertThrows(IllegalArgumentException.class, () -> CloseShieldChannel.wrap(channel, FileChannel.class)); + assertTrue(ex.getMessage().contains("not an interface")); + } + // Argument is not an instance of the specified interface. + // + // This situation is rare in normal code because the compiler enforces type compatibility + // when T is known at compile time. However, it can still occur at runtime if a value is + // passed through an unchecked cast or comes from a raw type, bypassing generic type checks. + try (Channel channel = mock(Channel.class)) { + @SuppressWarnings("rawtypes") + final Class channelClass = ReadableByteChannel.class; + @SuppressWarnings("unchecked") + final IllegalArgumentException ex = assertThrows(IllegalArgumentException.class, () -> CloseShieldChannel.wrap(channel, channelClass)); + assertTrue(ex.getMessage().contains("not an instance of")); + } + } } From f4c52d3c10f8999088d79b27de011edc46983254 Mon Sep 17 00:00:00 2001 From: "Piotr P. Karwasz" Date: Tue, 14 Oct 2025 19:12:06 +0200 Subject: [PATCH 2/7] Fixes interface discovery in `CloseShieldChannel` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR is split from #799. The `CloseShieldChannel` implementation only inspects interfaces **directly** implemented by the given channel’s class, ignoring those inherited from its superclasses. As a result, proxies for types such as `FileChannel` does not expose any of the interfaces declared on `FileChannel` itself. --- .../io/channels/CloseShieldChannel.java | 10 +++++--- .../io/channels/CloseShieldChannelTest.java | 24 +++++++++++++++++++ 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/apache/commons/io/channels/CloseShieldChannel.java b/src/main/java/org/apache/commons/io/channels/CloseShieldChannel.java index a9a462da73b..bde0feb25cc 100644 --- a/src/main/java/org/apache/commons/io/channels/CloseShieldChannel.java +++ b/src/main/java/org/apache/commons/io/channels/CloseShieldChannel.java @@ -40,11 +40,15 @@ public final class CloseShieldChannel { private static final Class[] EMPTY = {}; private static Set> collectChannelInterfaces(final Class type, final Set> out) { + Class currentType = type; // Visit interfaces - for (final Class iface : type.getInterfaces()) { - if (Channel.class.isAssignableFrom(iface) && out.add(iface)) { - collectChannelInterfaces(iface, out); + while (currentType != null) { + for (final Class iface : currentType.getInterfaces()) { + if (Channel.class.isAssignableFrom(iface) && out.add(iface)) { + collectChannelInterfaces(iface, out); + } } + currentType = currentType.getSuperclass(); } return out; } diff --git a/src/test/java/org/apache/commons/io/channels/CloseShieldChannelTest.java b/src/test/java/org/apache/commons/io/channels/CloseShieldChannelTest.java index ac7e85b7a89..42a23f0af1c 100644 --- a/src/test/java/org/apache/commons/io/channels/CloseShieldChannelTest.java +++ b/src/test/java/org/apache/commons/io/channels/CloseShieldChannelTest.java @@ -20,6 +20,7 @@ import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertNotSame; import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -32,11 +33,13 @@ import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; +import java.io.IOException; import java.nio.channels.AsynchronousByteChannel; import java.nio.channels.AsynchronousChannel; import java.nio.channels.ByteChannel; import java.nio.channels.Channel; import java.nio.channels.ClosedChannelException; +import java.nio.channels.FileChannel; import java.nio.channels.GatheringByteChannel; import java.nio.channels.InterruptibleChannel; import java.nio.channels.MulticastChannel; @@ -45,9 +48,12 @@ import java.nio.channels.ScatteringByteChannel; import java.nio.channels.SeekableByteChannel; import java.nio.channels.WritableByteChannel; +import java.nio.file.Path; import java.util.stream.Stream; +import org.apache.commons.io.FileUtils; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.MethodSource; @@ -280,4 +286,22 @@ void testWritableByteChannelMethods() throws Exception { assertThrows(ClosedChannelException.class, () -> shield.write(null)); verifyNoMoreInteractions(channel); } + + @Test + void testCorrectlyDetectsInterfaces(@TempDir Path tempDir) throws IOException { + final Path testFile = tempDir.resolve("test.txt"); + FileUtils.touch(testFile.toFile()); + try (FileChannel channel = FileChannel.open(testFile); Channel shield = CloseShieldChannel.wrap(channel)) { + assertInstanceOf(SeekableByteChannel.class, shield); + assertInstanceOf(GatheringByteChannel.class, shield); + assertInstanceOf(WritableByteChannel.class, shield); + assertInstanceOf(ScatteringByteChannel.class, shield); + assertInstanceOf(ReadableByteChannel.class, shield); + assertInstanceOf(InterruptibleChannel.class, shield); + assertInstanceOf(ByteChannel.class, shield); + assertInstanceOf(Channel.class, shield); + // These are not interfaces, so can not be implemented + assertFalse(shield instanceof FileChannel, "not FileChannel"); + } + } } From 7a0608f5b755e989930add275e3376d27cdb098c Mon Sep 17 00:00:00 2001 From: "Piotr P. Karwasz" Date: Tue, 14 Oct 2025 19:47:52 +0200 Subject: [PATCH 3/7] fix: add overloads for commons channel types --- .../io/channels/CloseShieldChannel.java | 43 ++++++++++++------- .../io/channels/CloseShieldChannelTest.java | 41 ++++-------------- 2 files changed, 36 insertions(+), 48 deletions(-) diff --git a/src/main/java/org/apache/commons/io/channels/CloseShieldChannel.java b/src/main/java/org/apache/commons/io/channels/CloseShieldChannel.java index 7be297e20bb..4f25cd48fb4 100644 --- a/src/main/java/org/apache/commons/io/channels/CloseShieldChannel.java +++ b/src/main/java/org/apache/commons/io/channels/CloseShieldChannel.java @@ -20,6 +20,9 @@ import java.io.Closeable; import java.lang.reflect.Proxy; import java.nio.channels.Channel; +import java.nio.channels.ReadableByteChannel; +import java.nio.channels.SeekableByteChannel; +import java.nio.channels.WritableByteChannel; import java.util.LinkedHashSet; import java.util.Objects; import java.util.Set; @@ -59,7 +62,7 @@ private static Set> collectChannelInterfaces(final Class type, final * @param channel The underlying channel to shield, not {@code null}. * @return A proxy that shields {@code close()} and enforces closed semantics on other calls. */ - @SuppressWarnings({ "unchecked", "resource" }) // caller closes + @SuppressWarnings({ "resource" }) // caller closes public static Channel wrap(final Channel channel) { Objects.requireNonNull(channel, "channel"); // Fast path: already our shield @@ -76,23 +79,31 @@ public static Channel wrap(final Channel channel) { /** * Wraps a channel to shield it from being closed. * - * @param channel The underlying channel to shield, not {@code null} and must implement {@code type}. - * @param type The interface the returned proxy must implement; - * the proxy will also implement all other {@link Channel} sub-interfaces that the delegate implements. - * @param A type that extends {@link Channel}. + * @param channel The underlying channel to shield, not {@code null}. * @return A proxy that shields {@code close()} and enforces closed semantics on other calls. - * @throws IllegalArgumentException if {@code type} is not an interface or if {@code channel} does not implement {@code type}. */ - @SuppressWarnings({ "unchecked", "resource" }) // caller closes - public static T wrap(final T channel, Class type) { - Objects.requireNonNull(type, "type"); - if (!type.isInterface()) { - throw new IllegalArgumentException(type.getName() + " is not an interface"); - } - if (!type.isInstance(channel)) { - throw new IllegalArgumentException("channel of type " + channel.getClass().getName() + " is not an instance of " + type.getName()); - } - return type.cast(wrap(channel)); + public static ReadableByteChannel wrap(final ReadableByteChannel channel) { + return (ReadableByteChannel) wrap((Channel) channel); + } + + /** + * Wraps a channel to shield it from being closed. + * + * @param channel The underlying channel to shield, not {@code null}. + * @return A proxy that shields {@code close()} and enforces closed semantics on other calls. + */ + public static WritableByteChannel wrap(final WritableByteChannel channel) { + return (WritableByteChannel) wrap((Channel) channel); + } + + /** + * Wraps a channel to shield it from being closed. + * + * @param channel The underlying channel to shield, not {@code null}. + * @return A proxy that shields {@code close()} and enforces closed semantics on other calls. + */ + public static SeekableByteChannel wrap(final SeekableByteChannel channel) { + return (SeekableByteChannel) wrap((Channel) channel); } private CloseShieldChannel() { diff --git a/src/test/java/org/apache/commons/io/channels/CloseShieldChannelTest.java b/src/test/java/org/apache/commons/io/channels/CloseShieldChannelTest.java index 6e15ba02bff..59fd1bc50a0 100644 --- a/src/test/java/org/apache/commons/io/channels/CloseShieldChannelTest.java +++ b/src/test/java/org/apache/commons/io/channels/CloseShieldChannelTest.java @@ -121,9 +121,9 @@ void testCloseIsShielded(final Class channelClass) throws Exc @Test void testDoesNotDoubleWrap() { - final ByteChannel channel = mock(ByteChannel.class); - final ByteChannel shield1 = CloseShieldChannel.wrap(channel, ByteChannel.class); - final ByteChannel shield2 = CloseShieldChannel.wrap(shield1, ByteChannel.class); + final Channel channel = mock(Channel.class); + final Channel shield1 = CloseShieldChannel.wrap(channel); + final Channel shield2 = CloseShieldChannel.wrap(shield1); assertSame(shield1, shield2); } @@ -143,7 +143,7 @@ void testEquals(final Class channelClass) throws Exception { void testGatheringByteChannelMethods() throws Exception { final GatheringByteChannel channel = mock(GatheringByteChannel.class); when(channel.isOpen()).thenReturn(true); - final GatheringByteChannel shield = CloseShieldChannel.wrap(channel, GatheringByteChannel.class); + final GatheringByteChannel shield = (GatheringByteChannel) CloseShieldChannel.wrap(channel); // Before close write() should delegate when(channel.write(null, 0, 0)).thenReturn(42L); assertEquals(42, shield.write(null, 0, 0)); @@ -168,7 +168,7 @@ void testHashCode(final Class channelClass) throws Exception void testNetworkChannelMethods() throws Exception { final NetworkChannel channel = mock(NetworkChannel.class); when(channel.isOpen()).thenReturn(true); - final NetworkChannel shield = CloseShieldChannel.wrap(channel, NetworkChannel.class); + final NetworkChannel shield = (NetworkChannel) CloseShieldChannel.wrap(channel); // Before close getOption(), setOption(), getLocalAddress() and bind() should delegate when(channel.getOption(null)).thenReturn("foo"); when(channel.setOption(null, null)).thenReturn(channel); @@ -207,7 +207,7 @@ void testPreservesInterfaces(final Class channelClass) { void testReadableByteChannelMethods() throws Exception { final ReadableByteChannel channel = mock(ReadableByteChannel.class); when(channel.isOpen()).thenReturn(true); - final ReadableByteChannel shield = CloseShieldChannel.wrap(channel, ReadableByteChannel.class); + final ReadableByteChannel shield = CloseShieldChannel.wrap(channel); // Before close read() should delegate when(channel.read(null)).thenReturn(42); assertEquals(42, shield.read(null)); @@ -222,7 +222,7 @@ void testReadableByteChannelMethods() throws Exception { void testScatteringByteChannelMethods() throws Exception { final ScatteringByteChannel channel = mock(ScatteringByteChannel.class); when(channel.isOpen()).thenReturn(true); - final ScatteringByteChannel shield = CloseShieldChannel.wrap(channel, ScatteringByteChannel.class); + final ScatteringByteChannel shield = (ScatteringByteChannel) CloseShieldChannel.wrap(channel); // Before close read() should delegate when(channel.read(null, 0, 0)).thenReturn(42L); assertEquals(42, shield.read(null, 0, 0)); @@ -237,7 +237,7 @@ void testScatteringByteChannelMethods() throws Exception { void testSeekableByteChannelMethods() throws Exception { final SeekableByteChannel channel = mock(SeekableByteChannel.class); when(channel.isOpen()).thenReturn(true); - final SeekableByteChannel shield = CloseShieldChannel.wrap(channel, SeekableByteChannel.class); + final SeekableByteChannel shield = CloseShieldChannel.wrap(channel); // Before close position() and size() should delegate when(channel.position()).thenReturn(42L); when(channel.size()).thenReturn(84L); @@ -276,7 +276,7 @@ void testToString(final Class channelClass) throws Exception void testWritableByteChannelMethods() throws Exception { final WritableByteChannel channel = mock(WritableByteChannel.class); when(channel.isOpen()).thenReturn(true); - final WritableByteChannel shield = CloseShieldChannel.wrap(channel, WritableByteChannel.class); + final WritableByteChannel shield = CloseShieldChannel.wrap(channel); // Before close write() should delegate when(channel.write(null)).thenReturn(42); assertEquals(42, shield.write(null)); @@ -304,27 +304,4 @@ void testCorrectlyDetectsInterfaces(@TempDir Path tempDir) throws IOException { assertFalse(shield instanceof FileChannel, "not FileChannel"); } } - - @Test - void testThrowsIllegalArgumentException(@TempDir Path tempDir) throws Exception { - final Path testFile = tempDir.resolve("test.txt"); - FileUtils.touch(testFile.toFile()); - // Argument is not an interface - try (FileChannel channel = FileChannel.open(testFile)) { - final IllegalArgumentException ex = assertThrows(IllegalArgumentException.class, () -> CloseShieldChannel.wrap(channel, FileChannel.class)); - assertTrue(ex.getMessage().contains("not an interface")); - } - // Argument is not an instance of the specified interface. - // - // This situation is rare in normal code because the compiler enforces type compatibility - // when T is known at compile time. However, it can still occur at runtime if a value is - // passed through an unchecked cast or comes from a raw type, bypassing generic type checks. - try (Channel channel = mock(Channel.class)) { - @SuppressWarnings("rawtypes") - final Class channelClass = ReadableByteChannel.class; - @SuppressWarnings("unchecked") - final IllegalArgumentException ex = assertThrows(IllegalArgumentException.class, () -> CloseShieldChannel.wrap(channel, channelClass)); - assertTrue(ex.getMessage().contains("not an instance of")); - } - } } From 4af33559dfe452c5034381b731affdc663a787c6 Mon Sep 17 00:00:00 2001 From: "Piotr P. Karwasz" Date: Tue, 14 Oct 2025 20:04:55 +0200 Subject: [PATCH 4/7] fix: add `ByteChannel` overload to resolve ambiguity --- .../commons/io/channels/CloseShieldChannel.java | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/main/java/org/apache/commons/io/channels/CloseShieldChannel.java b/src/main/java/org/apache/commons/io/channels/CloseShieldChannel.java index 4f25cd48fb4..df40e05cae1 100644 --- a/src/main/java/org/apache/commons/io/channels/CloseShieldChannel.java +++ b/src/main/java/org/apache/commons/io/channels/CloseShieldChannel.java @@ -19,6 +19,7 @@ import java.io.Closeable; import java.lang.reflect.Proxy; +import java.nio.channels.ByteChannel; import java.nio.channels.Channel; import java.nio.channels.ReadableByteChannel; import java.nio.channels.SeekableByteChannel; @@ -76,6 +77,16 @@ public static Channel wrap(final Channel channel) { set.isEmpty() ? new Class[] { Channel.class } : set.toArray(EMPTY), new CloseShieldChannelHandler(channel)); } + /** + * Wraps a channel to shield it from being closed. + * + * @param channel The underlying channel to shield, not {@code null}. + * @return A proxy that shields {@code close()} and enforces closed semantics on other calls. + */ + public static ByteChannel wrap(final ByteChannel channel) { + return (ByteChannel) wrap((Channel) channel); + } + /** * Wraps a channel to shield it from being closed. * From 28777bc154ddfd4519fc6869975e6fd1488e3e01 Mon Sep 17 00:00:00 2001 From: "Piotr P. Karwasz" Date: Tue, 14 Oct 2025 20:24:01 +0200 Subject: [PATCH 5/7] fix: Limit interfaces to those verified. --- .../io/channels/CloseShieldChannel.java | 7 ++-- .../channels/CloseShieldChannelHandler.java | 32 +++++++++++++++++++ .../io/channels/CloseShieldChannelTest.java | 5 --- 3 files changed, 35 insertions(+), 9 deletions(-) diff --git a/src/main/java/org/apache/commons/io/channels/CloseShieldChannel.java b/src/main/java/org/apache/commons/io/channels/CloseShieldChannel.java index df40e05cae1..c3459667b49 100644 --- a/src/main/java/org/apache/commons/io/channels/CloseShieldChannel.java +++ b/src/main/java/org/apache/commons/io/channels/CloseShieldChannel.java @@ -31,9 +31,8 @@ /** * Creates a close-shielding proxy for a {@link Channel}. * - *

- * The returned proxy will implement all {@link Channel} sub-interfaces that the delegate implements. - *

+ *

The returned proxy implements all {@link Channel} sub-interfaces that are both + * supported by this implementation and actually implemented by the given delegate.

* * @see Channel * @see Closeable @@ -48,7 +47,7 @@ private static Set> collectChannelInterfaces(final Class type, final // Visit interfaces while (currentType != null) { for (final Class iface : currentType.getInterfaces()) { - if (Channel.class.isAssignableFrom(iface) && out.add(iface)) { + if (CloseShieldChannelHandler.isSupported(iface) && out.add(iface)) { collectChannelInterfaces(iface, out); } } diff --git a/src/main/java/org/apache/commons/io/channels/CloseShieldChannelHandler.java b/src/main/java/org/apache/commons/io/channels/CloseShieldChannelHandler.java index f13b101c197..cca96ac76ec 100644 --- a/src/main/java/org/apache/commons/io/channels/CloseShieldChannelHandler.java +++ b/src/main/java/org/apache/commons/io/channels/CloseShieldChannelHandler.java @@ -21,14 +21,46 @@ import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.lang.reflect.Proxy; +import java.nio.channels.AsynchronousChannel; +import java.nio.channels.ByteChannel; import java.nio.channels.Channel; import java.nio.channels.ClosedChannelException; +import java.nio.channels.GatheringByteChannel; +import java.nio.channels.InterruptibleChannel; import java.nio.channels.NetworkChannel; +import java.nio.channels.ReadableByteChannel; +import java.nio.channels.ScatteringByteChannel; import java.nio.channels.SeekableByteChannel; +import java.nio.channels.WritableByteChannel; +import java.util.Collections; +import java.util.HashSet; import java.util.Objects; +import java.util.Set; final class CloseShieldChannelHandler implements InvocationHandler { + private static final Set> SUPPORTED_INTERFACES; + + static { + final Set> interfaces = new HashSet<>(); + interfaces.add(Channel.class); + interfaces.add(ReadableByteChannel.class); + interfaces.add(ScatteringByteChannel.class); + interfaces.add(WritableByteChannel.class); + interfaces.add(GatheringByteChannel.class); + interfaces.add(ByteChannel.class); + interfaces.add(SeekableByteChannel.class); + interfaces.add(NetworkChannel.class); + // Similar to Closeable + interfaces.add(AsynchronousChannel.class); + interfaces.add(InterruptibleChannel.class); + SUPPORTED_INTERFACES = Collections.unmodifiableSet(interfaces); + } + + static boolean isSupported(final Class interfaceClass) { + return SUPPORTED_INTERFACES.contains(interfaceClass); + } + /** * Tests whether the given method is allowed to be called after the shield is closed. * diff --git a/src/test/java/org/apache/commons/io/channels/CloseShieldChannelTest.java b/src/test/java/org/apache/commons/io/channels/CloseShieldChannelTest.java index 59fd1bc50a0..d2117a59dd0 100644 --- a/src/test/java/org/apache/commons/io/channels/CloseShieldChannelTest.java +++ b/src/test/java/org/apache/commons/io/channels/CloseShieldChannelTest.java @@ -34,7 +34,6 @@ import static org.mockito.Mockito.when; import java.io.IOException; -import java.nio.channels.AsynchronousByteChannel; import java.nio.channels.AsynchronousChannel; import java.nio.channels.ByteChannel; import java.nio.channels.Channel; @@ -42,7 +41,6 @@ import java.nio.channels.FileChannel; import java.nio.channels.GatheringByteChannel; import java.nio.channels.InterruptibleChannel; -import java.nio.channels.MulticastChannel; import java.nio.channels.NetworkChannel; import java.nio.channels.ReadableByteChannel; import java.nio.channels.ScatteringByteChannel; @@ -65,14 +63,11 @@ class CloseShieldChannelTest { static Stream> testedInterfaces() { // @formatter:off return Stream.of( - AsynchronousByteChannel.class, AsynchronousChannel.class, ByteChannel.class, Channel.class, GatheringByteChannel.class, InterruptibleChannel.class, - MulticastChannel.class, - NetworkChannel.class, NetworkChannel.class, ReadableByteChannel.class, ScatteringByteChannel.class, From 4d12b204425d3688d359d43d0d53ec96ad31959e Mon Sep 17 00:00:00 2001 From: "Piotr P. Karwasz" Date: Wed, 15 Oct 2025 00:03:08 +0200 Subject: [PATCH 6/7] fix: rollback previous test --- .../apache/commons/io/channels/CloseShieldChannelTest.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/java/org/apache/commons/io/channels/CloseShieldChannelTest.java b/src/test/java/org/apache/commons/io/channels/CloseShieldChannelTest.java index 84b5aa730df..e896df5928e 100644 --- a/src/test/java/org/apache/commons/io/channels/CloseShieldChannelTest.java +++ b/src/test/java/org/apache/commons/io/channels/CloseShieldChannelTest.java @@ -134,9 +134,9 @@ void testCorrectlyDetectsInterfaces(@TempDir Path tempDir) throws IOException { @Test void testDoesNotDoubleWrap() { - final Channel channel = mock(Channel.class); - final Channel shield1 = CloseShieldChannel.wrap(channel); - final Channel shield2 = CloseShieldChannel.wrap(shield1); + final ByteChannel channel = mock(ByteChannel.class); + final ByteChannel shield1 = CloseShieldChannel.wrap(channel); + final ByteChannel shield2 = CloseShieldChannel.wrap(shield1); assertSame(shield1, shield2); } From 2fc87cc79ff17b9919f722735001d09b237f38ac Mon Sep 17 00:00:00 2001 From: "Piotr P. Karwasz" Date: Thu, 16 Oct 2025 10:46:17 +0200 Subject: [PATCH 7/7] fix: Restore generic method --- .../io/channels/CloseShieldChannel.java | 73 +++++++------------ .../channels/CloseShieldChannelHandler.java | 13 ++-- .../io/channels/CloseShieldChannelTest.java | 6 +- 3 files changed, 37 insertions(+), 55 deletions(-) diff --git a/src/main/java/org/apache/commons/io/channels/CloseShieldChannel.java b/src/main/java/org/apache/commons/io/channels/CloseShieldChannel.java index c3459667b49..ba890d8a6a7 100644 --- a/src/main/java/org/apache/commons/io/channels/CloseShieldChannel.java +++ b/src/main/java/org/apache/commons/io/channels/CloseShieldChannel.java @@ -19,9 +19,14 @@ import java.io.Closeable; import java.lang.reflect.Proxy; +import java.nio.channels.AsynchronousChannel; import java.nio.channels.ByteChannel; import java.nio.channels.Channel; +import java.nio.channels.GatheringByteChannel; +import java.nio.channels.InterruptibleChannel; +import java.nio.channels.NetworkChannel; import java.nio.channels.ReadableByteChannel; +import java.nio.channels.ScatteringByteChannel; import java.nio.channels.SeekableByteChannel; import java.nio.channels.WritableByteChannel; import java.util.LinkedHashSet; @@ -31,8 +36,23 @@ /** * Creates a close-shielding proxy for a {@link Channel}. * - *

The returned proxy implements all {@link Channel} sub-interfaces that are both - * supported by this implementation and actually implemented by the given delegate.

+ *

The returned proxy implements all {@link Channel} sub-interfaces that are both supported by this implementation and actually implemented by the given + * delegate.

+ * + *

The following interfaces are supported:

+ * + *
    + *
  • {@link AsynchronousChannel}
    • + *
  • {@link ByteChannel}
    • + *
  • {@link Channel}
    • + *
  • {@link GatheringByteChannel}
    • + *
  • {@link InterruptibleChannel}
    • + *
  • {@link NetworkChannel}
    • + *
  • {@link ReadableByteChannel}
    • + *
  • {@link ScatteringByteChannel}
    • + *
  • {@link SeekableByteChannel}
    • + *
  • {@link WritableByteChannel}
    • + *
* * @see Channel * @see Closeable @@ -60,10 +80,13 @@ private static Set> collectChannelInterfaces(final Class type, final * Wraps a channel to shield it from being closed. * * @param channel The underlying channel to shield, not {@code null}. + * @param A supported channel type. * @return A proxy that shields {@code close()} and enforces closed semantics on other calls. + * @throws ClassCastException if {@code T} is not a supported channel type. + * @throws NullPointerException if {@code channel} is {@code null}. */ - @SuppressWarnings({ "resource" }) // caller closes - public static Channel wrap(final Channel channel) { + @SuppressWarnings({ "unchecked", "resource" }) // caller closes + public static T wrap(final T channel) { Objects.requireNonNull(channel, "channel"); // Fast path: already our shield if (Proxy.isProxyClass(channel.getClass()) && Proxy.getInvocationHandler(channel) instanceof CloseShieldChannelHandler) { @@ -72,50 +95,10 @@ public static Channel wrap(final Channel channel) { // Collect only Channel sub-interfaces. final Set> set = collectChannelInterfaces(channel.getClass(), new LinkedHashSet<>()); // fallback to root surface - return (Channel) Proxy.newProxyInstance(channel.getClass().getClassLoader(), // use delegate's loader + return (T) Proxy.newProxyInstance(channel.getClass().getClassLoader(), // use delegate's loader set.isEmpty() ? new Class[] { Channel.class } : set.toArray(EMPTY), new CloseShieldChannelHandler(channel)); } - /** - * Wraps a channel to shield it from being closed. - * - * @param channel The underlying channel to shield, not {@code null}. - * @return A proxy that shields {@code close()} and enforces closed semantics on other calls. - */ - public static ByteChannel wrap(final ByteChannel channel) { - return (ByteChannel) wrap((Channel) channel); - } - - /** - * Wraps a channel to shield it from being closed. - * - * @param channel The underlying channel to shield, not {@code null}. - * @return A proxy that shields {@code close()} and enforces closed semantics on other calls. - */ - public static ReadableByteChannel wrap(final ReadableByteChannel channel) { - return (ReadableByteChannel) wrap((Channel) channel); - } - - /** - * Wraps a channel to shield it from being closed. - * - * @param channel The underlying channel to shield, not {@code null}. - * @return A proxy that shields {@code close()} and enforces closed semantics on other calls. - */ - public static WritableByteChannel wrap(final WritableByteChannel channel) { - return (WritableByteChannel) wrap((Channel) channel); - } - - /** - * Wraps a channel to shield it from being closed. - * - * @param channel The underlying channel to shield, not {@code null}. - * @return A proxy that shields {@code close()} and enforces closed semantics on other calls. - */ - public static SeekableByteChannel wrap(final SeekableByteChannel channel) { - return (SeekableByteChannel) wrap((Channel) channel); - } - private CloseShieldChannel() { // no instance } diff --git a/src/main/java/org/apache/commons/io/channels/CloseShieldChannelHandler.java b/src/main/java/org/apache/commons/io/channels/CloseShieldChannelHandler.java index cca96ac76ec..a7f3d3694f1 100644 --- a/src/main/java/org/apache/commons/io/channels/CloseShieldChannelHandler.java +++ b/src/main/java/org/apache/commons/io/channels/CloseShieldChannelHandler.java @@ -43,17 +43,16 @@ final class CloseShieldChannelHandler implements InvocationHandler { static { final Set> interfaces = new HashSet<>(); + interfaces.add(AsynchronousChannel.class); + interfaces.add(ByteChannel.class); interfaces.add(Channel.class); + interfaces.add(GatheringByteChannel.class); + interfaces.add(InterruptibleChannel.class); + interfaces.add(NetworkChannel.class); interfaces.add(ReadableByteChannel.class); interfaces.add(ScatteringByteChannel.class); - interfaces.add(WritableByteChannel.class); - interfaces.add(GatheringByteChannel.class); - interfaces.add(ByteChannel.class); interfaces.add(SeekableByteChannel.class); - interfaces.add(NetworkChannel.class); - // Similar to Closeable - interfaces.add(AsynchronousChannel.class); - interfaces.add(InterruptibleChannel.class); + interfaces.add(WritableByteChannel.class); SUPPORTED_INTERFACES = Collections.unmodifiableSet(interfaces); } diff --git a/src/test/java/org/apache/commons/io/channels/CloseShieldChannelTest.java b/src/test/java/org/apache/commons/io/channels/CloseShieldChannelTest.java index e896df5928e..1f448f42cc2 100644 --- a/src/test/java/org/apache/commons/io/channels/CloseShieldChannelTest.java +++ b/src/test/java/org/apache/commons/io/channels/CloseShieldChannelTest.java @@ -156,7 +156,7 @@ void testEquals(final Class channelClass) throws Exception { void testGatheringByteChannelMethods() throws Exception { final GatheringByteChannel channel = mock(GatheringByteChannel.class); when(channel.isOpen()).thenReturn(true); - final GatheringByteChannel shield = (GatheringByteChannel) CloseShieldChannel.wrap(channel); + final GatheringByteChannel shield = CloseShieldChannel.wrap(channel); // Before close write() should delegate when(channel.write(null, 0, 0)).thenReturn(42L); assertEquals(42, shield.write(null, 0, 0)); @@ -181,7 +181,7 @@ void testHashCode(final Class channelClass) throws Exception void testNetworkChannelMethods() throws Exception { final NetworkChannel channel = mock(NetworkChannel.class); when(channel.isOpen()).thenReturn(true); - final NetworkChannel shield = (NetworkChannel) CloseShieldChannel.wrap(channel); + final NetworkChannel shield = CloseShieldChannel.wrap(channel); // Before close getOption(), setOption(), getLocalAddress() and bind() should delegate when(channel.getOption(null)).thenReturn("foo"); when(channel.setOption(null, null)).thenReturn(channel); @@ -235,7 +235,7 @@ void testReadableByteChannelMethods() throws Exception { void testScatteringByteChannelMethods() throws Exception { final ScatteringByteChannel channel = mock(ScatteringByteChannel.class); when(channel.isOpen()).thenReturn(true); - final ScatteringByteChannel shield = (ScatteringByteChannel) CloseShieldChannel.wrap(channel); + final ScatteringByteChannel shield = CloseShieldChannel.wrap(channel); // Before close read() should delegate when(channel.read(null, 0, 0)).thenReturn(42L); assertEquals(42, shield.read(null, 0, 0));