OkHttp Wishlist

[swankjesse]

This is an ongoing list of things we’d like to change by breaking API compatibility.

• Allow Cache-Control durations to be longs PR 2797
• OkHttpServer
• Merge ConnectionSpec and HandshakeCertificates, call it HandshakePlan
• Response’s body should be empty instead of null for the cacheResponse and networkResponse

[jaredsburrows]

You can check cache-control as the PR is now closed.

[JakeWharton]

It's closed because it's a breaking change to 3.x and it's unchecked on
this list because this is a list of breaking changes that we want.

On Sun, Oct 23, 2016, 9:05 PM Jared Burrows notifications@github.com
wrote:

You can check cache-control as the PR is now closed.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#2903 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAEEESl6fPA2iyQfNUXCqCaYGw6kwa0_ks5q3ARXgaJpZM4KUTnq
.

[gengjiawen]

I would to see ping pong api public available.
Related issue: #2902.

[JakeWharton]

That doesn't require a breaking change. Please file a separate issue with your use case.

…

On Fri, Dec 2, 2016, 1:31 AM Jiawen Geng ***@***.***> wrote: I would to see ping pong api public available. Related issue: #2902 <#2902>. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#2903 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAEEEam-rXznlXx-K9FMB2OhUYHQLO0Sks5rD7tPgaJpZM4KUTnq> .

[DanielNovak]

Implement support for "stale-while-revalidate" (#1083). This would be a great help for implementing very responsive apps without the need to have a custom DB cache (and usually some RxJava magic to deliver a cache response until the refreshed response from server is received). Though this may be quite difficult to implement since the response would be delivered twice :-/.

[commonsguy]

If you're taking requests... cache interceptors and/or pluggable storage implementations would help with security. IMHO, anywhere OkHttp writes to disk, ideally there is a hook for app developers to supply some other storage implementation (e.g., save the data to an already-existing encrypted data store, for HIPAA compliance and whatnot).

If there is a way that you would like me to help with those, let me know.

[swankjesse]

@commonsguy internally it’s pluggable via FileSystem.java. But that might not work as we go towards fancier things like Relay which will read and write simultaneously.

[gengjiawen]

Any possible we can poke on graphql ?
Okhttp of course can use graphql, but multiline string support isn't good in java, anything solution similar with sqldelight can be a good option.
And it would be nice to have auto-complete function if we provide if graphql schema file in Android Studio.(For js, we have js-graphql-intellij-plugin )

I am now handing graphql with retrofit, but it's a bit of awkward.

[JakeWharton]

@gengjiawen Get involved in the Apollo project? It aims to provide SQL Delight-like support for GraphQL over OkHttp.

[digitalbuddha]

Thanks for the plug, We're working on exactly what you are asking for over at Apollo-Android. Code gen for the messy parts, Retrofit for the networking. Come join the fun, there's a working sample already and AutoValue support is coming shortly. https://github.com/apollographql/apollo-android

[artem-zinnatullin]

Would be great to allow Source/BufferedSource as a body of chunked response in MockWebServer.

Currently MockWebServer only accepts Buffer/String which does not let you simulate responses with ability to write more data on the fly i.e. SSE over HTTP.

This will be breaking change for the MockResponse because it'll have to store body as Source/BufferedSource and that'll break MockResponse.body() behavior and return type or it could return null if body is not instance of Buffer and then it could be done in 3.x.

This was also mentioned in OkHttpServer issue but if you're going to have both MockWebServer and OkHttpServer this seems to belong to the 4.0 Wishlist.

[artem-zinnatullin]

Another thing that probably deserves separate issue for discussion is async non-blocking Interceptors API based on callbacks rather than on synchronous method calls. This would work great with reactive libraries.

Currently if custom Interceptor can block Thread for relatively long time (i.e. session token is not ready yet so request can not be executed) the option to avoid that is to remove this logic from Interceptor and write it somewhere closer to application business logic which can greatly complicate things.

[NoahAndrews]

I would really like to see WebSocketListener be an interface rather than an abstract class. Maybe have an abstract version for convenience. Why was the decision made for it to be an abstract class in the first place?

[swankjesse]

It's too late to change. If we were to change it in OkHttp 4, what’s the benefit of using an interface?

[swankjesse]

Dispatcher’s maxRequestsPerHost setting is more appropriate for HTTP/1.1. We may want to revisit for HTTP/2.

[allen026]

I wish can receive Server Sent Event in OKHttp 4.0

[JakeWharton]

You can do that currently if you parse the body format yourself. And we could provide a parser in 3.x. I don't think there's anything tied to 4.0 for that request.

…

On Thu, Sep 7, 2017 at 5:08 AM Allen ***@***.***> wrote: I wish can receive Server Sent Event in OKHttp 4.0 — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#2903 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAEEEfH-SdLwmKv4yfRsbHmzEf4EgVJOks5sf7J6gaJpZM4KUTnq> .

[akosma]

@swankjesse answering your question to @NoahAndrews (June 15th 2017) the benefit of an abstract class over an interface would be (and this is just an example) that you could make an Android service to be "WebSocketListener" (given the lack of multiple inheritance, because an Android service already has to extend from another base class). Right now one needs to create a separate class, make it inherit from WebSocketListener, instantiate it, and wire it all together in a service, und so weiter.
In the current state, the fact that WebSocketListener is an abstract class with pure virtual methods without any implementation whatsoever does not seem to bring any visible added value to the table, and it's hard to see why it could not be an interface instead. But then again, maybe I'm missing something that's not immediately obvious, and would love to learn more; this is not a criticism, just an observation.

[iTimeTraveler]

Is there a good way to implement the non-blocking I/O (i.e. java.nio or epoll or I/O
Multiplexing) to save the number of threads when concurrently requesting, while preserving the interceptor design pattern? I like this design and API.

[swankjesse]

Coroutines!

[gengjiawen]

kotlin ?

[artem-zinnatullin]

For coroutines or other means of thread pooling to be helpful, Interceptors API will need to be non-blocking. Otherwise, interceptor can block execution thread for long time thus causing delays for other requests if thread pool is limited or causing additional thread spawns.

See also #2903 (comment)

[mandrachek]

@commonsguy internally it’s pluggable via FileSystem.java. But that might not work as we go towards fancier things like Relay which will read and write simultaneously.

@swankjesse So... I'm able to use reflection to create an "EncryptingFileSystem.kt", and pass it into my builder, like so:

val okHttpClient = OkHttpClient.Builder()
        .cache(getCache(File(context.cacheDir.absolutePath + "/okhttp.encrypted"),10*1024*1024, key))
        .build()

This works, writing the iv to a separate file, and skipping encryption of the journal (not sure how appendingSink would interact), using the standard java cipher streams. (I was using CipherSink/CipherSource, but it appears there's some kind of issue that triggers Android's IV re-use check, along with forgetting to call response.close() in my test).

I'm sure you're aware this isn't exactly a new request (#1605), so it would be really great if you could design and possibly test for this kind of use case and make the API for it public with 5.

[swankjesse]

@mandrachek I’m surprised that doesn’t work. Our tests for this stuff all go through a fake file system. If there’s anything other than “non-public API” here I’d love to fix it for you immediately.

[mandrachek]

@swankjesse Sorry, I edited my post about the same time as you replied - it actually does work. I wasn't calling response.close(), combine that with an android keystore incompatibility/bug in CipherSink/CipherSource (complaining about IV re-use)...and well, that was broken. I replaced CipherSink/CipherSource with CipherOutputStream/CipherInputStream, and ensured close() was being called, and it works now!

Posted a gist with the EncryptingFileSystem implementation. So, nothing wrong with Cache or FileSystem, was on my end. I exempted the journal from being encrypted - I don't think there's any benefit to encrypting it, and it's the only place that uses appendingSink, which I'm not sure would work well with the cipher streams.

[swankjesse]

@mandrachek great! Agreed on not encrypting the journal. The URLs you hit are stored hashed in the journal, but they’re also on the file system directly. An attacker who has access to your cache but not your private key will be able to figure out what URLs you’ve visited.

What do you think about OkHttp offering an overload of Cache that takes a symmetric key? We could do encrypting internally and end-users like yourself would only need to provide the key. Our implementation would probably be similar to yours – wrap a file system to apply encryption.

[mandrachek]

@swankjesse There's all sorts of factors at play with encryption that might make a one-size fits all approach not work so well - for example I'm using AES/GCM/NoPadding, generating my own 12byte IV (required for this transformation), while others may need to use a different transformation, a different sized IV (or use an IV generated automatically by the cipher), a different SecureRandom, a specific non-default JCE provider, or even asymmetric encryption.

This method is nice (to me anyway) because it's flexible and lets me leverage.the JCE -- I can use keys from the AndroidKeyStore where the actual key material is not exposed, unlike Realm, which does the encryption itself without the JCE (embedded native OpenSSL AES for better performance, using byte array as the key), and so which requires some hoop jumping (encrypt the key using key from AKS, store it on disk, decrypt with key from keystore, and pass to realm, or some similar set of steps).

Guess I could see you going that way too though, to optimize performance.

[swankjesse]

Yeah. I think good security mostly comes by making it easy to do the right thing. Right now ~zero caches are encrypted. If we made it super easy, it’d be a lot more.

[maiph]

Have you consider using Jetty or something of sorts or java.nio for the OkHttpServer ?

[yschimke]

@swankjesse is this list actively curated?

[swankjesse]

My current preference is to ‘never’ do an OkHttp release that breaks compatibility with OkHttp 3.x. If we were to take on such a release, we’d want to have very strong motivation for introducing such damage because we’d start out with 0 users on the new okhttp5 package name.

The purpose of this list is to capture those motivations and make the case for that change. (So far I don’t think there’s enough compelling stuff in 5.x that we can’t do in 4.x.)

[iNoles]

How about combine Call.Factory and WebSocket.Factory?

[JakeWharton]

What does that gain?

…

On Sun, Jul 5, 2020, at 10:46 AM, Jonathan wrote: How about combine Call.Factory and WebSocket.Factory? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#2903 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAQIEN6FBTNCUPXBQIIVALR2CG5XANCNFSM4CSRHHVA>.

[iNoles]

What does that gain?
…
On Sun, Jul 5, 2020, at 10:46 AM, Jonathan wrote: How about combine Call.Factory and WebSocket.Factory? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#2903 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAQIEN6FBTNCUPXBQIIVALR2CG5XANCNFSM4CSRHHVA.

Well, there are a lot of libraries are using the kotlin's lazy function on Call.Factory, then they could not use websocket by default. Retrofit is also using Call.Factory too. If you choose to have one factory class for Call and WebSocket, it would be much helpful.

[JakeWharton]

Do you have example libraries? Why don't they reference the OkHttpClient type?

Retrofit accepts either an OkHttpClient or a Call.Factory, but since it doesn't support websockets that's fine. Even if it did, we would just add a function that would take a WebSocket.Factory and passing an OkHttpClient would set both.

[iNoles]

coil-kt, coil-kt/coil#8

[JakeWharton]

I'm not really seeing a problem that would have been solved by unifying these interfaces in that link.

…

On Sun, Jul 5, 2020, at 6:08 PM, Jonathan wrote: coil-kt, coil-kt/coil#8 <coil-kt/coil#8> — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#2903 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAQIENRACFVUQTNM74RMOTR2D2XRANCNFSM4CSRHHVA>.

[yogurtearl]

Would vote for this to be on the wishlist.

can have more in-flight HTTP calls than threads

from
square/okio#531 (comment)

[yschimke]

Renamed to a general wishlist, 5.0 blockers are here

#7366

[AlexTrotsenko]

Yeah. I think good security mostly comes by making it easy to do the right thing. Right now ~zero caches are encrypted. If we made it super easy, it’d be a lot more.

I would like just to double-check if currently there any plans for making cached data to be somehow encrypted?
Btw, do you think we can somehow consider using EncryptedFile from androidx.security:security-crypto ?

[yschimke]

Yeah. I think good security mostly comes by making it easy to do the right thing. Right now ~zero caches are encrypted. If we made it super easy, it’d be a lot more.

I would like just to double-check if currently there any plans for making cached data to be somehow encrypted? Btw, do you think we can somehow consider using EncryptedFile from androidx.security:security-crypto ?

Cache exposes the okio FileSystem param. So consider implementing this yourself.

https://github.com/square/okhttp/pull/6550/files

[mandrachek]

I have implemented it. You just have to put it in the same namespace to get around the base class being private. Ultimately my implementation wound up being a bit different than the gist, as I found AndroidKeystore has thread safety issues.

A jetpack dependency is most likely out of the question, and even if it wasn't, the EncryptedFile would not work. The cache requires saving data to a temp file, and then moving/renaming to the final destination. EncryptedFile uses the file name and path, and will fail to decrypt the file if the name or path has changed!

[yschimke]

Closing in favour of other more specific issues.