Skip to content

core: handle empty proxy selector result#12793

Open
mfperminov wants to merge 1 commit into
grpc:masterfrom
mfperminov:add-check-for-proxies-empty-list
Open

core: handle empty proxy selector result#12793
mfperminov wants to merge 1 commit into
grpc:masterfrom
mfperminov:add-check-for-proxies-empty-list

Conversation

@mfperminov
Copy link
Copy Markdown

Fixes a crash in ProxyDetectorImpl when ProxySelector.select(URI) returns an empty list.

On some Android devices/configurations, ProxySelector.select(uri) can return
Collections.emptyList(). ProxyDetectorImpl previously assumed the returned list
always contained at least one element and called proxies.get(0), causing:

 Fatal Exception: java.lang.IndexOutOfBoundsException: Index: 0
       at java.util.Collections$EmptyList.get(Collections.java:4888)
       at io.grpc.internal.ProxyDetectorImpl.detectProxy(ProxyDetectorImpl.java:243)
       at io.grpc.internal.ProxyDetectorImpl.proxyFor(ProxyDetectorImpl.java:202)
       at io.grpc.internal.DnsNameResolver.detectProxy(DnsNameResolver.java:269)
       at io.grpc.internal.DnsNameResolver.access$600(DnsNameResolver.java:66)
       at io.grpc.internal.DnsNameResolver$Resolve.run(DnsNameResolver.java:310)

This change treats null/empty proxy selector results as "no proxy" and proceeds
without proxy, matching the behavior for Proxy.Type.DIRECT.

Added a unit test covering an empty proxy list.

@linux-foundation-easycla
Copy link
Copy Markdown

linux-foundation-easycla Bot commented May 4, 2026

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: mfperminov / name: Mikhail Perminov (ac43fae)

@mfperminov mfperminov force-pushed the add-check-for-proxies-empty-list branch 3 times, most recently from b6066e0 to da37275 Compare May 4, 2026 20:57
@mfperminov mfperminov force-pushed the add-check-for-proxies-empty-list branch from da37275 to ac43fae Compare May 4, 2026 21:25
@mfperminov
Copy link
Copy Markdown
Author

Hi @ejona86, would you be the right person to review this small proxy detection fix?

@ejona86
Copy link
Copy Markdown
Member

ejona86 commented May 13, 2026

Can we identify the broken Android deployments? I know we can't fix them, but we can document what software we are working around. The Java API says there will always be entries in the list, so both null and empty list are invalid. I'd normally suggest we fail (cleanly, not like it is now) instead of ignoring the API breakage.

@mfperminov
Copy link
Copy Markdown
Author

Can we identify the broken Android deployments? I know we can't fix them, but we can document what software we are working around. The Java API says there will always be entries in the list, so both null and empty list are invalid. I'd normally suggest we fail (cleanly, not like it is now) instead of ignoring the API breakage.

Thanks for the context.
I unfortunately cannot provide detailed production/device information because of NDA. The only information I can share is that, in our production crash reports, more than 90% of the occurrences are on Android 14 devices, across multiple manufacturers.
Given that the Java API contract requires the returned list to be non-empty, I understand your concern about silently treating this as "no proxy".
If I understand your suggestion correctly, the preferred direction would be to fail explicitly and add a comment explaining the workaround/behavior, e.g. replace the current accidental IndexOutOfBoundsException with a clearer exception such as IllegalStateException when ProxySelector returns an empty list.
Is that the approach you would prefer for this PR?

@ejona86
Copy link
Copy Markdown
Member

ejona86 commented May 13, 2026

So I've looked around some, and I don't see any other reports of something like this. I'm very suspicious, as Android 14 is plenty old enough for us to have heard of this problem. It is seeming more likely at this point that maybe you have a broken ProxySelector configured somewhere in your app.

How does this sound: Add in checks for null/isEmpty(), and throw an IOException with the classname of the ProxySelector as part of the error string. I know exception strings are often discarded on Android, but this has no semantic concerns and would help anyone that accidentally writes a broken ProxySelector. Ideally the ProxySelector would get fixed, instead of all callers working around it.

If we find out this behavior is widespread we would re-evaluate the approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants