okhttp

okhttp copied to clipboard

We should implement user-requested protocol upgrades as specified by RFC 7230 section 6.7.

Use Cases

• Docker connection hijacking see also docker-java #1347
• Our own web sockets
• Third-party web sockets implementations on OkHttp

Eligibility

An HTTP/1 call is upgraded if all of the following are true:

• The caller’s request includes an Upgrade header
• The caller’s request includes a Connection header with the value upgrade
• The server’s response includes an Upgrade header
• The server’s response includes a Connection header with the value upgrade
• The response code is 101.

On Upgrade

A successful upgrade changes the behavior of the HTTP response:

• The ResponseBody is null.
• The response has a non-null Streams object that carries the input and output stream. Note that the source and sink timeouts should work properly!

interface Streams {
  val source: BufferedSource
  val sink: BufferedSink
  fun cancel()
}

class Response {
  ...
  /** Non-null if this response is a successful upgrade ... */
  @get:JvmName("streams") val streams: Streams?
}

A successful upgrade has these side-effects:

• The call timeout is immediately completed. (RealCall.timeoutEarlyExit())
• The connection is forbidden from carrying new exchanges. (RealConnection.noNewExchanges())
• The socket’s read timeout is disabled. (Socket.setSoTimeout())
• If the call is asynchronous (Call.enqueue()), the call counts against Dispatcher limits until onResponse() returns.

I’ve used the class name Streams instead of UpgradedConnection or something feature-specific because I think we might be able to reuse this type for CONNECT calls.

Web Sockets

Can we migrate our internal web sockets code to use this? Ideally yes, though that shouldn’t block this from being released.

Event Listeners

Ideally we have well defined behavior for EventListeners on an upgraded connection. We need to decide whether to count bytes of the upgraded connection for the benefit of listeners.

What are the cancellation & close rules? Half closed supported?

Source.close(): releases resources and cause writes to fail. Sink.close(): flushes buffered data, releases resources, and sends EOF to reader. cancel(): asynchronously cause source & sink to fail, each on both ends.

Is this one still under consideration? Any kind of official support for a connection upgrade and exposing Sink/Source would be great for OkHttp 4+.

Yeah I'd like for us to do this.

@gesellix what's your use case?

@swankjesse thanks for coming back to this one!

It's the same use case like for docker-java (Docker's HTTP Hijacking), with the only difference that I'm maintaining yet another Java docker-client, based on OkHttp 4.x. You can find myself struggling with a hacky implementation.

I'm now trying to have a more generic implementation, based on Docker's Swagger definition and more generated code - with the hijacking feature being one of the custom parts, obviously, and I didn't want to copy the old hacky code over to the fresh implementation.

Maybe this one helps: I tried to implement integration test by copying the relevant code from Docker to a small utility. It's certainly not ready for wide-spread use and it probably needs some polishing, but you might also have the problem of testing the hijacking client.

@swankjesse do you think this one could be included in OkHttp 5? I'm aware that the required changes could change the API, so it probably won't be backported to OkHttp 4 - which is fine.

Yeah, this is something we want.

Is it worth landing this for 4.9? before we do the right fix in 5.0 and then backport?

https://github.com/square/okhttp/pull/7196

With Cloudflare sending these, it seems more timely for 4.9?

I started working on this one. You're welcome to leave comments/suggestions at https://github.com/gesellix/okhttp/pull/4. Maybe it will become good enough to be a pull request to the main repository?

The biggest question I have is, should Okio define the Streams type?

A big +1. I think I've been asking for that for a while.

Worth getting the API right.

Maybe there are good ways to tie it in with Kotlin coroutine scopes? Is kotlinx-io doing anything here?