Okio 2.x incompatibilities

[Kernald]

What version of gRPC-Java are you using?

1.36.0

What is your environment?

Linux and macOS, built with Bazel from sources using the tools provided in this repo, rather than through a Maven dependency. The same repo contains an Android app using OkHttp3 (4.9.1), which depends on Okio 2.8+.

What did you expect to see?

gRPC-Java should build just fine.

What did you see instead?

gRPC-Java either overrides the dependencies and force Okio 1.x (which causes issues with OkHttp3, e.g. square/okhttp#5378), or fails to compile when using Okio 2.8+. For example:

grpc-java/okhttp/src/main/java/io/grpc/okhttp/OkHttpReadableBuffer.java

Line 43 in b2e4757

return buffer.readByte() & 0x000000FF;

buffer.readByte() now throws an exception, which causes a compilation error as it's not caught or thrown by readUnsignedByte(). This points to incompatibilities between Okio 1.x and 2.x, and makes adopting gRPC-Java in any repo using "recent" (mid-2019...) versions of OkHttp an issue. OkHttp3 has been depending on Okio 2.x since its 4.0.0 release.

Steps to reproduce the bug

Depend on both OkHttp3 4.x and gRPC-Java at the same time.

[ejona86]

Catching that exception and re-throwing as IndexOutOfBoundsException is a small change and with it all our tests pass. So I don't think there's any serious incompatibility here; I think you can just use newer okio versions without trouble.

Note that this readByte() case is called out specifically in okio CHANGELOG, but that it is ABI-compatible. Given our code should never be triggering the exception. the difference shouldn't matter all that much.

Were you just building from source to determine the issues with upgrading? If so, then I'd say there are none and you are free to bump the version for gRPC at-will.

Tested with:

diff --git a/build.gradle b/build.gradle
index 5c605dcab..d9bb7336e 100644
--- a/build.gradle
+++ b/build.gradle
@@ -156,7 +156,7 @@ subprojects {
             google_auth_credentials: "com.google.auth:google-auth-library-credentials:${googleauthVersion}",
             google_auth_oauth2_http: "com.google.auth:google-auth-library-oauth2-http:${googleauthVersion}",
             okhttp: 'com.squareup.okhttp:okhttp:2.7.4',
-            okio: 'com.squareup.okio:okio:1.17.5',
+            okio: 'com.squareup.okio:okio:2.10.0',
             opencensus_api: "io.opencensus:opencensus-api:${opencensusVersion}",
             opencensus_contrib_grpc_metrics: "io.opencensus:opencensus-contrib-grpc-metrics:${opencensusVersion}",
             opencensus_impl: "io.opencensus:opencensus-impl:${opencensusVersion}",
diff --git a/okhttp/src/main/java/io/grpc/okhttp/OkHttpReadableBuffer.java b/okhttp/src/main/java/io/grpc/okhttp/OkHttpReadableBuffer.java
index 821f2e5fd..7aa74dd91 100644
--- a/okhttp/src/main/java/io/grpc/okhttp/OkHttpReadableBuffer.java
+++ b/okhttp/src/main/java/io/grpc/okhttp/OkHttpReadableBuffer.java
@@ -40,7 +40,11 @@ class OkHttpReadableBuffer extends AbstractReadableBuffer {
 
   @Override
   public int readUnsignedByte() {
-    return buffer.readByte() & 0x000000FF;
+    try {
+      return buffer.readByte() & 0x000000FF;
+    } catch (EOFException e) {
+      throw new IndexOutOfBoundsException(e.getMessage());
+    }
   }
 
   @Override

[ejona86]

I also tested with throw new AssertionError() and there weren't any problems.

[Kernald]

Thanks for the (very) quick feedback on that! I'm using gRPC-Java in a Bazel repo, and have been since before the initial release of rules_jvm_external so I've been building it from sources since then (using @io_grpc_grpc_java//:repositories.bzl:grpc_java_repositories() and, more recently, the dependencies and overrides provided for rules_jvm_external which are what pulled Okio 1.x). I have no problem personally switching to fetching gRPC-Java through Maven, so it's indeed a non-blocking issue.

Out of curiosity, is bumping the Okio version pulled by gRPC-Java something that could be considered and/or done?

[ejona86]

I'm using gRPC-Java in a Bazel repo

Oh, okay. Then we'll need to get this fixed up. Since EOFException is a checked exception build will fail if we continue using okio 1.x in our build. We can workaround that by making a trash function that just "throws EOFException" and calling it within the try.

Out of curiosity, is bumping the Okio version pulled by gRPC-Java something that could be considered and/or done?

I've considered it on multiple occasions. The additional dependencies would be a bit of a pain as users pick them up, but could be dealt with. But I sort of hate looking through the Kotlin code (I can't read it) so I've not been eager to personally push that upgrade forward.

[ejona86]

built with Bazel from sources using the tools provided in this repo,

Doh. I didn't read the initial description closely enough to notice that. Sorry. We're quite happy to have Bazel users build from source.

[Kernald]

No worries, thanks!