Skip to content

Commit 08573a6

Browse files
authored
Fixes issues in CloseShieldChannel (#799)
* Fixes issues in `CloseShieldChannel` 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<T>)`, 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. * Fixes interface discovery in `CloseShieldChannel` 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. * fix: add overloads for commons channel types * fix: add `ByteChannel` overload to resolve ambiguity * fix: Limit interfaces to those verified. * fix: rollback previous test * fix: Restore generic method
1 parent f51d19c commit 08573a6

File tree

3 files changed

+61
-10
lines changed

3 files changed

+61
-10
lines changed

src/main/java/org/apache/commons/io/channels/CloseShieldChannel.java

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,17 +19,40 @@
1919

2020
import java.io.Closeable;
2121
import java.lang.reflect.Proxy;
22+
import java.nio.channels.AsynchronousChannel;
23+
import java.nio.channels.ByteChannel;
2224
import java.nio.channels.Channel;
25+
import java.nio.channels.GatheringByteChannel;
26+
import java.nio.channels.InterruptibleChannel;
27+
import java.nio.channels.NetworkChannel;
28+
import java.nio.channels.ReadableByteChannel;
29+
import java.nio.channels.ScatteringByteChannel;
30+
import java.nio.channels.SeekableByteChannel;
31+
import java.nio.channels.WritableByteChannel;
2332
import java.util.LinkedHashSet;
2433
import java.util.Objects;
2534
import java.util.Set;
2635

2736
/**
2837
* Creates a close-shielding proxy for a {@link Channel}.
2938
*
30-
* <p>
31-
* The returned proxy will implement all {@link Channel} sub-interfaces that the delegate implements.
32-
* </p>
39+
* <p>The returned proxy implements all {@link Channel} sub-interfaces that are both supported by this implementation and actually implemented by the given
40+
* delegate.</p>
41+
*
42+
* <p>The following interfaces are supported:</p>
43+
*
44+
* <ul>
45+
* <li>{@link AsynchronousChannel}</li>
46+
* <li>{@link ByteChannel}</li>
47+
* <li>{@link Channel}</li>
48+
* <li>{@link GatheringByteChannel}</li>
49+
* <li>{@link InterruptibleChannel}</li>
50+
* <li>{@link NetworkChannel}</li>
51+
* <li>{@link ReadableByteChannel}</li>
52+
* <li>{@link ScatteringByteChannel}</li>
53+
* <li>{@link SeekableByteChannel}</li>
54+
* <li>{@link WritableByteChannel}</li>
55+
* </ul>
3356
*
3457
* @see Channel
3558
* @see Closeable
@@ -44,7 +67,7 @@ private static Set<Class<?>> collectChannelInterfaces(final Class<?> type, final
4467
// Visit interfaces
4568
while (currentType != null) {
4669
for (final Class<?> iface : currentType.getInterfaces()) {
47-
if (Channel.class.isAssignableFrom(iface) && out.add(iface)) {
70+
if (CloseShieldChannelHandler.isSupported(iface) && out.add(iface)) {
4871
collectChannelInterfaces(iface, out);
4972
}
5073
}
@@ -57,8 +80,10 @@ private static Set<Class<?>> collectChannelInterfaces(final Class<?> type, final
5780
* Wraps a channel to shield it from being closed.
5881
*
5982
* @param channel The underlying channel to shield, not {@code null}.
60-
* @param <T> Any Channel type (interface or class).
83+
* @param <T> A supported channel type.
6184
* @return A proxy that shields {@code close()} and enforces closed semantics on other calls.
85+
* @throws ClassCastException if {@code T} is not a supported channel type.
86+
* @throws NullPointerException if {@code channel} is {@code null}.
6287
*/
6388
@SuppressWarnings({ "unchecked", "resource" }) // caller closes
6489
public static <T extends Channel> T wrap(final T channel) {

src/main/java/org/apache/commons/io/channels/CloseShieldChannelHandler.java

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,45 @@
2121
import java.lang.reflect.InvocationTargetException;
2222
import java.lang.reflect.Method;
2323
import java.lang.reflect.Proxy;
24+
import java.nio.channels.AsynchronousChannel;
25+
import java.nio.channels.ByteChannel;
2426
import java.nio.channels.Channel;
2527
import java.nio.channels.ClosedChannelException;
28+
import java.nio.channels.GatheringByteChannel;
29+
import java.nio.channels.InterruptibleChannel;
2630
import java.nio.channels.NetworkChannel;
31+
import java.nio.channels.ReadableByteChannel;
32+
import java.nio.channels.ScatteringByteChannel;
2733
import java.nio.channels.SeekableByteChannel;
34+
import java.nio.channels.WritableByteChannel;
35+
import java.util.Collections;
36+
import java.util.HashSet;
2837
import java.util.Objects;
38+
import java.util.Set;
2939

3040
final class CloseShieldChannelHandler implements InvocationHandler {
3141

42+
private static final Set<Class<?>> SUPPORTED_INTERFACES;
43+
44+
static {
45+
final Set<Class<?>> interfaces = new HashSet<>();
46+
interfaces.add(AsynchronousChannel.class);
47+
interfaces.add(ByteChannel.class);
48+
interfaces.add(Channel.class);
49+
interfaces.add(GatheringByteChannel.class);
50+
interfaces.add(InterruptibleChannel.class);
51+
interfaces.add(NetworkChannel.class);
52+
interfaces.add(ReadableByteChannel.class);
53+
interfaces.add(ScatteringByteChannel.class);
54+
interfaces.add(SeekableByteChannel.class);
55+
interfaces.add(WritableByteChannel.class);
56+
SUPPORTED_INTERFACES = Collections.unmodifiableSet(interfaces);
57+
}
58+
59+
static boolean isSupported(final Class<?> interfaceClass) {
60+
return SUPPORTED_INTERFACES.contains(interfaceClass);
61+
}
62+
3263
/**
3364
* Tests whether the given method is allowed to be called after the shield is closed.
3465
*

src/test/java/org/apache/commons/io/channels/CloseShieldChannelTest.java

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,15 +34,13 @@
3434
import static org.mockito.Mockito.when;
3535

3636
import java.io.IOException;
37-
import java.nio.channels.AsynchronousByteChannel;
3837
import java.nio.channels.AsynchronousChannel;
3938
import java.nio.channels.ByteChannel;
4039
import java.nio.channels.Channel;
4140
import java.nio.channels.ClosedChannelException;
4241
import java.nio.channels.FileChannel;
4342
import java.nio.channels.GatheringByteChannel;
4443
import java.nio.channels.InterruptibleChannel;
45-
import java.nio.channels.MulticastChannel;
4644
import java.nio.channels.NetworkChannel;
4745
import java.nio.channels.ReadableByteChannel;
4846
import java.nio.channels.ScatteringByteChannel;
@@ -65,14 +63,11 @@ class CloseShieldChannelTest {
6563
static Stream<Class<? extends Channel>> testedInterfaces() {
6664
// @formatter:off
6765
return Stream.of(
68-
AsynchronousByteChannel.class,
6966
AsynchronousChannel.class,
7067
ByteChannel.class,
7168
Channel.class,
7269
GatheringByteChannel.class,
7370
InterruptibleChannel.class,
74-
MulticastChannel.class,
75-
NetworkChannel.class,
7671
NetworkChannel.class,
7772
ReadableByteChannel.class,
7873
ScatteringByteChannel.class,

0 commit comments

Comments
 (0)