add connection header to excluded sigv4 headers

[shulin-sq]

context: #2594

https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Connection

connection is a hop by hop header that is expected to be stripped by the server. It is already excluded in the java sdk and should be excluded here as well.

[lucix-aws]

What exactly does this fix? I get that Java is doing it but it's still a behavior change for us. Behavior changes, especially around signing, come with risk. An explanation or really a concrete example of why this matters helps us to be sure we're doing the right thing here.

[shulin-sq]

@lucix-aws hello thank you for the response. As per the issue/discussion post I linked, this was a legitimate issue we ran into when trying to perform sigv4a signing with this library.

I liked an explanation from my coworker which I am going to steal: "the reason Connection should not be included in signature calculations is because it is a hop-by-hop header. that means that individual hops between your client and the aws service api (e.g. forward proxies, CDNs, reverse proxies, etc) are expected to drop the header. so even if the header was sent by your client, it wasn't received by the aws server that did signature validation"

I do wish that the java sdk left a bit more documentation around why this header was excluded so that this is easier to understand.

[shulin-sq]

@lucix-aws are there any other concerns?

[lucix-aws]

Yes - your justification is completely sound on spec, and if I was releasing this today I would elect to include this change, but as it stands I still don't understand what this is actually fixing. Based on some rudimentary local testing we don't appear to be setting a Connection header in the SDKs to begin with.

Behavior changes, especially around signing, come with risk

To expound - this change in behavior affects everybody using the v2 SDK or its sigv4 implementation. We have no real way of knowing whether something out there that consumes this software would be broken by the newly unsigned/un-canonicalized header. I would really rather not touch signing in this way unless absolutely necessary.

As it stands if I took this patch I would be assuming the risk without any understanding of the benefit.