Update "insecure" client tests for SNI client

[ptaoussanis]

PR #513 switches the default http-kit client to an SNI-capable when on Java >= 8.

Unfortunately this change breaks unit tests that rely on http-kit's trustAnybody SSL engine.

Rather than hold up the merge of #513 (and so the first v2.7 beta), I've temporarily disabled the failing tests.

It would be good to get these tests fixed before the v2.7 RC, assistance very welcome!

[gpind]

This broke us in production. IMO this either should've been a blocker for 2.7, or else the version should've been bumped to 3.0.

[ptaoussanis]

@gpind Hi Michael, I'm very sorry about the problem! This is entirely on me, I'd incorrectly concluded that the affected tests were vestigial since they use an :insecure? flag that doesn't seem to be documented as part of http-kit's public API.

Could you please share more details on exactly what happened in your case? How exactly were you affected? What kind of solution/workaround was necessary? Anything relevant you can share might be helpful.

or else the version should've been bumped to 3.0.

While it's easy to miss and not relevant in this case since the breakage was unintended, I'll note for future reference that http-kit uses Break Versioning - so the version bump to 2.7 is intended to indicate the possibility of minor breaks.

In any case, I'll note that since http-kit lost its author several years ago - it's currently maintained by its community. While we do the best we can, errors undoubtedly will slip in from time to time. Realistically, more than in the average author-led project since we're all pretty strapped for time, and none of us is deeply familiar with the whole codebase or its design or history. I would recommend testing new releases before deploying to production.

I'll add additional guidance on this to future release notes.

Finally, just to reiterate- I really am sorry for any unintended breaks, I know how much stress that can cause. My sincere apologies.

[ptaoussanis]

Update: I just found a reference to the :insecure? option on the legacy website. I'll add a warning to the release notes for anyone that might be using this, and create an issue to restore support

[ptaoussanis]

Have updated the CHANGELOG with appropriate warnings, and created a dedicated issue ([#528]) for the regression.

@gpind Thanks a lot for pinging about this!

[gpind]

Thanks for being understanding, @ptaoussanis. This stuff happens.

Could you please share more details on exactly what happened in your case? How exactly were you affected? What kind of solution/workaround was necessary? Anything relevant you can share might be helpful.

We communicate with many legacy servers we don't control. In some cases we talk to them via a proxy over VPN, and the setup is such that we need to pass :insecure? to http-kit (I'm not familiar with the details of that setup). When we bumped http-kit to 2.7, our requests to some of these servers started throwing errors like this:

"Hostname or IP address is undefined."
:via
[{:type javax.net.ssl.SSLHandshakeException
  :message "Hostname or IP address is undefined."
  :at [sun.security.ssl.Alert createSSLException "Alert.java" 131]}
 {:type java.security.cert.CertificateException
  :message "Hostname or IP address is undefined."
  :at [sun.security.util.HostnameChecker match "HostnameChecker.java" 97]}]
:trace
[[sun.security.util.HostnameChecker match "HostnameChecker.java" 97]
 [sun.security.ssl.X509TrustManagerImpl checkIdentity "X509TrustManagerImpl.java" 461]
 [sun.security.ssl.X509TrustManagerImpl checkIdentity "X509TrustManagerImpl.java" 435]
 [sun.security.ssl.AbstractTrustManagerWrapper checkAdditionalTrust "SSLContextImpl.java" 1566]
 [sun.security.ssl.AbstractTrustManagerWrapper checkServerTrusted "SSLContextImpl.java" 1507]
 [sun.security.ssl.CertificateMessage$T12CertificateConsumer checkServerCerts "CertificateMessage.java" 632]
 [sun.security.ssl.CertificateMessage$T12CertificateConsumer onCertificate "CertificateMessage.java" 473]
 [sun.security.ssl.CertificateMessage$T12CertificateConsumer consume "CertificateMessage.java" 369]
 [sun.security.ssl.SSLHandshake consume "SSLHandshake.java" 392]
 [sun.security.ssl.HandshakeContext dispatch "HandshakeContext.java" 443]
 [sun.security.ssl.SSLEngineImpl$DelegatedTask$DelegatedAction run "SSLEngineImpl.java" 1076]
 [sun.security.ssl.SSLEngineImpl$DelegatedTask$DelegatedAction run "SSLEngineImpl.java" 1063]
 [java.security.AccessController doPrivileged "AccessController.java" -2]
 [sun.security.ssl.SSLEngineImpl$DelegatedTask run "SSLEngineImpl.java" 1010]
 [org.httpkit.client.HttpsRequest doHandshake "HttpsRequest.java" 91]
 [org.httpkit.client.HttpClient doRead "HttpClient.java" 220]
 [org.httpkit.client.HttpClient run "HttpClient.java" 521]
 [java.lang.Thread run "Thread.java" 829]]

The immediate fix was simply to downgrade back to the version of http-kit we were on before.

We of course could and should have caught this ourselves, and we're looking into why we didn't.

I hadn't heard of Break Versioning, and will keep it in mind from now on. Thank you!

[ptaoussanis]

Likewise - thanks a lot for your understanding, and for the additional info!

The immediate fix was simply to downgrade back to the version of http-kit we were on before.

That's probably the best bet in the meantime, especially if you're otherwise satisfied with the previous version. Will try prioritise #528 after my next batch of open-source work, and update here if there's any news.

[ptaoussanis]

Closing since this should now be addressed in v2.8.0-beta1, Ref. #528 (comment).

Again apologies for the trouble.