Fix regression: the new default SNI client doesn't support :insecure? req option

[ptaoussanis]

Context

• http-kit 2.3.0 added an optional SNI-capable client for Client feature request: add SNI support #187.

• http-kit 2.7.0 switched to using this client by default for SNI support to be enabled by default? #393.

• The switch broke some client unit tests, prompting the creation of Update "insecure" client tests for SNI client #519.

• Since the broken tests seemed to rely on an undocumented :insecure? flag, and since there'd been no reports of issues in a couple months of pre-release usage - I concluded that the flag was likely vestigial and Update "insecure" client tests for SNI client #519 need not hold up the release of 2.7.0.

Unfortunately it seems I was mistaken, and there was at least one public mention of the :insecure? flag in the past - meaning that some users may already depend on the feature.

At least one user was unfortunately affected.

Options

First up, I'll add a warning now to the current release notes that the client :insecure? feature is likely broken with the default 2.7.0 client.

Beyond that, we'll need to decide if we want to retain support for :insecure? or not.

• If yes:

  • We'll need someone to volunteer a fix.
  • The disabled tests at Update "insecure" client tests for SNI client #519 can then be reenabled.
  • We can get the feature properly documented as officially supported.

• If no:

  • We can pull out the vestigial code+tests and reaffirm the warning.

Input welcome, I'm not too familiar off-hand with the purpose of this flag - and am occupied on some other topics atm.

[ptaoussanis]

Quick thought: while I'm not sure off-hand what the prior/usual semantics of an "insecure" option are meant to be, one option that could be worth considering is just reverting back to the old (pre-SNI) client when :insecure? is true.

Something like that might be a reasonable + easy stop-gap that'd help us at least restore the previous functionality without too much delay?

Again, would need to be investigated a bit. From digging through past issues, it looks like the original :insecure? feature was added here and motivated by comparison to clj-http, so we can probably check their semantics for reference.

If no one else is available to assist, I'll try take a look at this myself the next time I'm doing batched work on http-kit.

[ptaoussanis]

This has been (at least partially) addressed in v2.8.0-beta1.

The previous tests have been reinstated, and :insecure? now gets the exact same behaviour as it did in the past.

The remaining question is if there'd be any benefit for :insecure? to instead be implemented natively by the new SNI client. Feedback welcome if anyone has any thoughts on that, otherwise I think this can be closed in the meantime since at the first least the regression should now be fixed.