Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gRPC-okhttp ipv6 link local scope #9325

Closed
jader-eero opened this issue Jun 29, 2022 · 9 comments · Fixed by #9326 or #11725
Closed

gRPC-okhttp ipv6 link local scope #9325

jader-eero opened this issue Jun 29, 2022 · 9 comments · Fixed by #9326 or #11725
Assignees
Labels
Milestone

Comments

@jader-eero
Copy link
Contributor

What version of gRPC-Java are you using?

1.45.0

What is your environment?

MacOS jdk 8

What did you expect to see?

I expect gRPC-okhttp be able to work with ipv6 link local with scope

What did you see instead?

AndroidNegotiator inside OkHttpProtocolNegotiator.java class is using SNI_HOST_NAME.newInstance(hostname) which throws exception because SNIHostName(String hostname) doesn't accept "%" and we need that for scoping ipv6 link local.

Suggestion would be using SNIHostName(byte[] encoded). To do so we would need to replace line 252 to SNI_HOST_NAME.newInstance(hostname.getBytes())

Steps to reproduce the bug

Any connection that uses configureTlsExtensions from AndroidNegotiator class which uses ipv6 link local with scoping like "%wlan0"

@sanjaypujare
Copy link
Contributor

The constructor was looked up (using reflection) as Class.forName("javax.net.ssl.SNIHostName").getConstructor(String.class) on line 197. That will need to be changed to look up a constructor with byte[] type before your suggested change can be made.

Could you create a PR after testing your change works?

@jader-eero
Copy link
Contributor Author

@sanjaypujare I checked locally and that was the issue, I was able to connect successfully after making your and mine suggestion. I'll open a PR soon. Thank you.

@jader-eero
Copy link
Contributor Author

PR open #9326

@jader-eero
Copy link
Contributor Author

@sanjaypujare could you help me with the PR checks failing? I'm not familiar with them.

@sanjaypujare
Copy link
Contributor

with the PR checks failing

Which checks are failing?

@jader-eero
Copy link
Contributor Author

I was able to solve, thank you. Waiting approval from the reviewer.

@ejona86 ejona86 linked a pull request Jul 8, 2022 that will close this issue
@ejona86 ejona86 closed this as completed Jul 8, 2022
@ejona86 ejona86 added the bug label Jul 8, 2022
@ejona86 ejona86 added this to the 1.48 milestone Jul 8, 2022
@ejona86 ejona86 reopened this Oct 4, 2022
@ejona86 ejona86 removed this from the 1.48 milestone Oct 4, 2022
@ejona86 ejona86 self-assigned this Feb 3, 2023
@ejona86 ejona86 added this to the Next milestone Feb 3, 2023
@larry-safran larry-safran assigned larry-safran and unassigned ejona86 Feb 6, 2023
@ejona86 ejona86 assigned shivaspeaks and unassigned larry-safran Jun 24, 2024
@ejona86
Copy link
Member

ejona86 commented Sep 10, 2024

Reverted in #9375

@shivaspeaks
Copy link
Member

The reverted PR mentioned above says,

It produced compilation issues inside Google. I strongly suspect it isn't
this commit or gRPC's fault, but it prevents further testing until it is
resolved.

I see that the blaze build is successful in google3 with the fix that was reverted.
!InetAddresses.isInetAddress(HostAndPort.fromString(hostname).getHost())

Perhaps, the compilation issue was not gRPC's fault? 🧐 @ejona86

@ejona86
Copy link
Member

ejona86 commented Nov 12, 2024

It wasn't gRPC's fault. But something using gRPC on Android had a build that couldn't depend on (IIRC) HostAndPort because of funny business they were doing with Guava or something.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
5 participants