HTTPS proxy support

[itning]

Currently, it is my understanding that okhttp supports two types of proxys: HTTP/SOCKS, which are the proxy types built into JDK, but the vast majority of the use cases involve HTTPS proxies; any plans to support them?

[itning]

Apache http client can support HTTPS proxy:

HttpHost proxy = new HttpHost("https","www.xxx.xxx",8888);
CloseableHttpClient client = HttpClients.custom()
                    .setProxy(proxy)
                    .build();

So can OKHTTP client support it also?

[yschimke]

I think we should support. I've set up tests to validate against other proxily servers but haven't been able to easily verify Https behaviour.

Do you have a public test server I can test against? That you know is working ?

[itning]

I think we should support. I've set up tests to validate against other proxily servers but haven't been able to easily verify Https behaviour.

Do you have a public test server I can test against? That you know is working ?

Thanks so much for your reply. I don’t currently have a publicly available proxy for you to test; however this is the project we are working from to set up our proxy. I hope this is somewhat helpful.

[yschimke]

I confirmed with smokescreen that this clumsy workaround works

  val serverCert = """
      -----BEGIN CERTIFICATE-----
      MIIE+DCCAuCgAwIBAgIBBTANBgkqhkiG9w0BAQsFADBrMQswCQYDVQQGEwJVUzET
...
      B5Ird7f+U0bSPpszX4PMGbydhsuhnbqDamrT0Q==
      -----END CERTIFICATE-----
    """.trimIndent().decodeCertificatePem()
    
    val certs = HandshakeCertificates.Builder()
      .addPlatformTrustedCertificates()
      .addTrustedCertificate(serverCert)
      .build()

    val sslSocketFactory = certs.sslSocketFactory()

    val client = OkHttpClient.Builder()
      .proxy(Proxy(Proxy.Type.HTTP, InetSocketAddress("localhost", 4750)))
      .socketFactory(sslSocketFactory)
      .build()

[yschimke]

OK change in code is simple, except our dependence on java.net.Proxy is limiting.

@swankjesse would it make sense to have our own model for Proxy?

[yschimke]

A quick exploration, I'll revert some of this and see if it looks ok

#8377

[itning]

Hi @yschimke @swankjesse, Do we have release date for this feature?

[yschimke]

I don't think clean support will make 5.0, as it would require a new Proxy domain model.

Instead I'm aiming to allow this workaround #8379

[itning]

@yschimke Do you mean we can have a workaround method without changing the source code?

[yschimke]

@itning No, would require changing the client.

How are you configuring this proxy? Maybe I can see if there is a way to make that path work.

[yschimke]

We don't currently have a similar API to Apache

HttpHost proxy = new HttpHost("https","www.xxx.xxx",8888);

[itning]

How are you configuring this proxy? Maybe I can see if there is a way to make that path work.

For now we use apache http client like this: #8373 (comment)

[yschimke]

This would require this clumsy workaround for OkHttp

#8373 (comment)

Or at minimum

   // create a ssl socket factory somehow
    val sslSocketFactory = OkHttpClient().sslSocketFactory

    val client = OkHttpClient.Builder()
      .proxy(Proxy(Proxy.Type.HTTP, InetSocketAddress("localhost", 4750)))
      .socketFactory(sslSocketFactory)
      .build()

[itning]

// create a ssl socket factory somehow
val sslSocketFactory = OkHttpClient().sslSocketFactory

val client = OkHttpClient.Builder()
  .proxy(Proxy(Proxy.Type.HTTP, InetSocketAddress("localhost", 4750)))
  .socketFactory(sslSocketFactory)
  .build()

I use the code, but got error:

Exception in thread "main" java.lang.IllegalArgumentException: socketFactory instanceof SSLSocketFactory
	at okhttp3.OkHttpClient$Builder.socketFactory(OkHttpClient.kt:723)
	at xxx.main(xxx.java:178)

[yschimke]

Sorry, I should have been clearer.

It requires this PR, unless you wrap that socket factory again.

#8379

[itning]

@yschimke I'm sorry to bother you again and request when I can use the solution you provided?

[yschimke]

For now, try something like this

      val client =
        OkHttpClient.Builder()
          .sslSocketFactory(socketFactory, trustManager)
          .proxy(Proxy(Proxy.Type.HTTP, it.remoteAddress()))
          .protocols(listOf(Protocol.HTTP_2, Protocol.HTTP_1_1))
          .socketFactory(socketFactory.asSocketFactory())
          .build()

  private fun SSLSocketFactory.asSocketFactory(): SocketFactory {
    return DelegatingSocketFactory(this)
  }

DelegatingSocketFactory would be something like

okhttp/okhttp-testing-support/src/main/kotlin/okhttp3/DelegatingSocketFactory.kt

Line 27 in d0b6a46

open class DelegatingSocketFactory(private val delegate: SocketFactory) : SocketFactory() {

[Endeavour233]

For now, try something like this

      val client =
        OkHttpClient.Builder()
          .sslSocketFactory(socketFactory, trustManager)
          .proxy(Proxy(Proxy.Type.HTTP, it.remoteAddress()))
          .protocols(listOf(Protocol.HTTP_2, Protocol.HTTP_1_1))
          .socketFactory(socketFactory.asSocketFactory())
          .build()

  private fun SSLSocketFactory.asSocketFactory(): SocketFactory {
    return DelegatingSocketFactory(this)
  }

DelegatingSocketFactory would be something like

okhttp/okhttp-testing-support/src/main/kotlin/okhttp3/DelegatingSocketFactory.kt

Line 27 in d0b6a46

open class DelegatingSocketFactory(private val delegate: SocketFactory) : SocketFactory() {

Excuse me, it might not be the right place to ask this question, but I really felt confused about it and found nothing useful after googling my question. Why the protocols(protocols: List<Protocol>) method is designed in a way that h2_prior_knowledge can not coexist with other protocols. What if some of my calls are applicable for cleartext http/2 while others are applicable for http/2? Thank you.

// OKHttpClient.kt
     *
     * @param protocols the protocols to use, in order of preference. If the list contains
     *     [Protocol.H2_PRIOR_KNOWLEDGE] then that must be the only protocol and HTTPS URLs will not
     *     be supported. Otherwise the list must contain [Protocol.HTTP_1_1]. The list must
     *     not contain null or [Protocol.HTTP_1_0].
     */
    fun protocols(protocols: List<Protocol>) =

[yschimke]

https://datatracker.ietf.org/doc/html/rfc7540#section-3.4

The client and server must start the http/2 handshake without prior negotiation. So this would corrupt if the server expects plaintext or vice versa.

You could still support https/2 over https on a different port, but you can clone the client for that case.

Supporting a mix seems really niche and problematic.

[Endeavour233]

https://datatracker.ietf.org/doc/html/rfc7540#section-3.4

The client and server must start the http/2 handshake without prior negotiation. So this would corrupt if the server expects plaintext or vice versa.

You could still support https/2 over https on a different port, but you can clone the client for that case.

Supporting a mix seems really niche and problematic.

Thanks. Sounds great to clone the client.

However, I didn't really understand what you mean by http/2 handshake. A followup question if you can spare the time:

Suppose that I set protocols to be Protocol.HTTP_2, Protocol.H2_PRIOR_KNOWLEDGE and that I have known host_cleartext supports HTTP/2 cleartext, I use the client to request the resource http://host_cleartext/xxx and https://other_host/yyy.

I thought that h2_prior_knowledge would be applied to my first request, and that after negotiating with other_host through ALPN, either http/1.1 or http/2 over TLS(i.e. Protocol.HTTP_2, not sure whether my understanding of this Protocol enum is right) would be applied to my second request. Both requests would be processed as expected. I can't see where would corrupt.

Probably, issues would rise If I want to send HTTP/1.1 request, like http://host1/xxx, and HTTP/2 cleartext request, like http://host_cleartext/xxx, using one client with protocols Protocol.HTTP_1_1, Protocol.H2_PRIOR_KOWNLEDGE? Because the client can't tell which request is http/2 cleartext applicable to.

[yschimke]

Yep, it could be made to work that way. But in general, I don't think it's clear what everyone would expect.

Enabling this but only as a 100% toggle was probably simpler.

[Endeavour233]

Yep, it could be made to work that way. But in general, I don't think it's clear what everyone would expect.

Enabling this but only as a 100% toggle was probably simpler.

Yep, it could be made to work that way. But in general, I don't think it's clear what everyone would expect.

Enabling this but only as a 100% toggle was probably simpler.

Yep, it could be made to work that way. But in general, I don't think it's clear what everyone would expect.

Enabling this but only as a 100% toggle was probably simpler.

Got it, thank you~