OkHttp: extract hijacking logic into HijackingInterceptor - #1347

@yschimke would love to hear your thoughts on it if/when you have time 😊

I mean you are definitely off-piste here. I'm quite impressed by the code. The non reflection version looks a lot better, but still uses internal non-supported APIs. So worth landing, since it's less bad :)

Not sure what you want me to say. I would apologise in advance that we will accidentally break this at some point, except that we probably won't give you are on 3.14.x.

You should probably confirm a few things like HTTP/1.1, or does it actually work over h2 as well?

Ultimately it gives me a lot of hope for the h2-websocket support lysine-dev/okhttp#3918, but not sure that is really a thing yet in the wild.

FWIW I've written similar to support RSocket over HTTP/2 with Jetty myself, and this looks cleaner.

https://github.com/rsocket/rsocket-cli/blob/master/src/main/kotlin/io/rsocket/cli/http2/Http2DuplexConnection.kt
https://github.com/rsocket/rsocket-cli/blob/master/src/main/kotlin/io/rsocket/cli/http2/Http2ClientTransport.kt

@yschimke Thanks for having a look! 👍

I absolutely understand that I am using internal API here and that at some moment of time it may break 😅

You should probably confirm a few things like HTTP/1.1, or does it actually work over h2 as well?

It is all 1.1, just Docker does a very nasty thing they call "connection hijacking" where they "upgrade" you to "tcp" and you continue using the input stream to send stdin. They have websocket version of one of these endpoints, but not all, unfortunately.

Thanks for the pointers, I will definitely have a look!

To be clear I meant your code looks better than my shitty version

@yschimke thanks :D

BTW: wow! I didn't even know that you can do something like that with Jersey, last time I tried I ended up deleting my WIP code and googling "good http library for JVM", which of course led me to OkHttp :D

Would you accept a follow-up PR that makes this work on OkHttp 4.x also?

(at least until we figure out a proper API for upgrade calls)

Hi @swankjesse,

Thanks for joining the discussion, it is great have you here!

Do I understand you correctly that it won't work with OkHttp 4 (due to our use of the Internal class) but there is a way to make it work with OkHttp 4? If so, I would be glad to adjust this PR to make it work with both.

We shade OkHttp in Testcontainers, but not in docker-java (at least not yet), and it is indeed a good point that we should think about the potential of having OkHttp 4 on classpath, thanks for pointing out 👍

AFAIR GitHub allows submitting PRs to non-master branches, so that you can submit it to the add_okhttp_HijackingInterceptor upstream branch.
You can also point to the "correct" internal API in version 4 and I will adjust it accordingly :)

btw, having a public API for doing this (especially in 3.x branch!) would be a Christmas gift that came early! 😍

Superseeded by #1351