Improve Proxy protocol to support fallback to non-Proxy connections transparently

[stevenschlansker]

• Rework ProxyV?Connection to handoff buffers via UpgradeFrom/UpgradeTo rather than a bespoke constructor
• Add HttpConnection UpgradeTo support
• ProxyConnectionFactory now allows a fallback for non-Proxy protocol connections

TODO:

• SslConnection needs UpgradeTo support, but how do I test it? It seems that SslSocket doesn't allow you to send unencrypted data, which makes it difficult to create the necessary mixed unencrypted / encrypted data stream

This is a work in progress asking for feedback, please do not merge just yet!

[stevenschlansker]

This change was rejected on the jetty-dev mailing list 😢

[joakime]

UpgradeFrom and UpgradeTo are interfaces for HTTP Upgrade (to websocket and h2c).
Not really appropriate for Proxy.

[stevenschlansker]

On Apr 11, 2016, at 8:27 AM, Joakim Erdfelt notifications@github.com wrote:

UpgradeFrom and UpgradeTo are interfaces for HTTP Upgrade (to websocket and h2c).
Not really appropriate for Proxy.

Maybe it was never intended to have this use case, but it did work
extremely well. I was able to complete support for Proxy protocol
testing whether the stream has the proper header, and if not,
hand it off to the usual connector.

The setup worked with little fuss, and we completed our migration
with a minimum of configuration changes, saving our staff
a fair amount of time.

Totally understand if you do not want it upstream, but I found it very useful.

[gregw]

I've reopen this as I want to have bit more of a discussion about it.
Part of the concern about Proxy falling back to HTTP was that the implementation was likely to be ugly. However, you've done an excellent job of using the APIs that exist to make a PR that has very little impact on the code and none on performance.

Another concern is that this goes in an opposite direction for what we are doing with HTTP/2. Namely that the HTTP/1 connector can detect a PRI method and upgrade to HTTP/2. It may be a better idea to follow that pattern, although that is more likely to have performance impacts.

Also need to think about the SSL issues you raise.

Ultimately our concern is complexity for a very small use-case for which there are other work arounds.

So we will probably still reject this, but thought I'd open it up again for a bit more discussion just to make sure we are not throwing out a good idea.

[gregw]

OK I'm closing this again as we didn't provoke any more discussion. It is a good patch looking for a better use-case to justify it (or something similar). Thanks.

[nhartner]

This is an excellent feature that should be added. As it stands right now, you cannot upgrade a non-proxy LB + non-proxy jetty server to a proxy-LB + proxy jetty server without having downtime (in a multi node environment where jetty upgrades are rolled out 1 node at a time). The only way it can be done right now is to shut down the LB and all jetty nodes, apply upgrades and restart. Not acceptable for a zero-downtime rollout.

[sbordet]

See discussion: https://dev.eclipse.org/mhonarc/lists/jetty-dev/msg02682.html.

[ksperling]

While the migration scenario is one use case, we've got a setup where we want to make an initial connection through a load balancer, but also allow clients to connect directly to those instances. In this scenario it would be very desirable to be able to use a single connector to handle both the proxy and direct connections rather than having to run them on different ports.

[ksperling]

Hm on further mediation, optional proxy support would allow the "direct" clients to send afake proxy header... maybe different ports is the way to go after all.