Skip to content

Commit 4111f6f

Browse files
authored
core: throw IOException when ProxySelector returns null or empty list (#12793)
ProxySelector.select(URI) is contractually required to return a non-null, non-empty list. Some implementations violate this, which previously caused an opaque crash in ProxyDetectorImpl: ``` java.lang.IndexOutOfBoundsException: Index: 0 at java.util.Collections$EmptyList.get at io.grpc.internal.ProxyDetectorImpl.detectProxy ``` Detect this case explicitly and throw an IOException naming the offending ProxySelector class, so a broken implementation can be identified and fixed by its author rather than silently worked around in every caller.
1 parent 9410caf commit 4111f6f

2 files changed

Lines changed: 31 additions & 0 deletions

File tree

core/src/main/java/io/grpc/internal/ProxyDetectorImpl.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,14 @@ private ProxiedSocketAddress detectProxy(InetSocketAddress targetAddr) throws IO
207207
}
208208

209209
List<Proxy> proxies = proxySelector.select(uri);
210+
// ProxySelector.select(URI) is contractually required to return a non-null, non-empty list.
211+
// Surface the offending implementation's class name so a broken ProxySelector can be fixed.
212+
if (proxies == null || proxies.isEmpty()) {
213+
throw new IOException(
214+
"ProxySelector " + proxySelector.getClass().getName()
215+
+ " returned " + (proxies == null ? "null" : "an empty list")
216+
+ ", which violates the java.net.ProxySelector#select(URI) contract");
217+
}
210218
if (proxies.size() > 1) {
211219
log.warning("More than 1 proxy detected, gRPC will select the first one");
212220
}

core/src/test/java/io/grpc/internal/ProxyDetectorImplTest.java

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import static org.junit.Assert.assertNotEquals;
2222
import static org.junit.Assert.assertNotNull;
2323
import static org.junit.Assert.assertNull;
24+
import static org.junit.Assert.assertThrows;
2425
import static org.junit.Assert.assertTrue;
2526
import static org.mockito.ArgumentMatchers.any;
2627
import static org.mockito.ArgumentMatchers.anyInt;
@@ -33,13 +34,15 @@
3334
import io.grpc.HttpConnectProxiedSocketAddress;
3435
import io.grpc.ProxiedSocketAddress;
3536
import io.grpc.ProxyDetector;
37+
import java.io.IOException;
3638
import java.net.InetAddress;
3739
import java.net.InetSocketAddress;
3840
import java.net.PasswordAuthentication;
3941
import java.net.Proxy;
4042
import java.net.ProxySelector;
4143
import java.net.SocketAddress;
4244
import java.net.URI;
45+
import java.util.Collections;
4346
import org.junit.Before;
4447
import org.junit.Rule;
4548
import org.junit.Test;
@@ -191,4 +194,24 @@ public ProxySelector get() {
191194
authenticator);
192195
assertNull(proxyDetector.proxyFor(destination));
193196
}
197+
198+
@Test
199+
public void throwsWhenProxySelectorReturnsEmptyList() throws Exception {
200+
when(proxySelector.select(any(URI.class))).thenReturn(Collections.<Proxy>emptyList());
201+
202+
IOException e =
203+
assertThrows(IOException.class, () -> proxyDetector.proxyFor(destination));
204+
assertTrue(e.getMessage(), e.getMessage().contains("empty list"));
205+
assertTrue(e.getMessage(), e.getMessage().contains(proxySelector.getClass().getName()));
206+
}
207+
208+
@Test
209+
public void throwsWhenProxySelectorReturnsNullList() throws Exception {
210+
when(proxySelector.select(any(URI.class))).thenReturn(null);
211+
212+
IOException e =
213+
assertThrows(IOException.class, () -> proxyDetector.proxyFor(destination));
214+
assertTrue(e.getMessage(), e.getMessage().contains("null"));
215+
assertTrue(e.getMessage(), e.getMessage().contains(proxySelector.getClass().getName()));
216+
}
194217
}

0 commit comments

Comments
 (0)