Support HTTP/1.1 protocol upgrades

swankjesse commented 4 years ago

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.

yschimke commented 4 years ago

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

swankjesse commented 4 years ago

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.

gesellix commented 2 years ago

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+.

swankjesse commented 2 years ago

Yeah I'd like for us to do this.

swankjesse commented 2 years ago

@gesellix what's your use case?

gesellix commented 2 years ago

@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.

gesellix commented 2 years ago

@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.

swankjesse commented 2 years ago

Yeah, this is something we want.

yschimke commented 2 years ago

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?

gesellix commented 8 months ago

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?

swankjesse commented 6 months ago

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

yschimke commented 6 months ago

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?

square / okhttp