fix multi-value headers concatenation

[bentsku]

Motivation

We had a bug report for a service using the SimpleRequestsClient proxying HTTP response back to the user that multiple Set-Cookie headers got concatenated into one header. This is the default behavior of requests because requests.Response does not handle multi values headers.

In order to support this, we need to access the raw urlllib3 response which still holds the raw multi values headers. This is a similar issue than localstack/localstack#12577, which got somewhat the same way for the testing behavior.
As werkzeug.Headers supports multi value natively, this is not too hard to support.

Changes

• use the raw urllib3 response headers to build the Werkzeug response Headers
• add a small test showcasing the behavior

[cloutierMat]

That all looks good to me, Thanks for tackling this 🚀

I have one questions before I approve, since, at first glance, it seems like a pretty impactful change. Do you think that this change can have negative side effects for consumers of the SimpleRequestsClient. Where a concatenated string would be expected instead of a list for other headers? I am sure that upstream tests would reveal but I am still curious if it could impact users code beyond the Set-Cookie header. 🤔

[bentsku]

I have one questions before I approve, since, at first glance, it seems like a pretty impactful change. Do you think that this change can have negative side effects for consumers of the SimpleRequestsClient. Where a concatenated string would be expected instead of a list for other headers? I am sure that upstream tests would reveal but I am still curious if it could impact users code beyond the Set-Cookie header. 🤔

I think if end users of the product had this kind of response, it was wrong and would have been different in AWS, so I'd class this in the "parity" kind of fix.

But for the consumers of the SimpleRequestsClient, they should be able to handle proper Werkzeug Headers, and usually will return the response directly as is and would not manipulate the response of the SimpleRequestsClient as it most often used as a Proxy.

I'd say this is a pretty major flaw we have now that needs to be fixed, even if breaking some consumers. I would also argue that this is the advertised behavior of the Werkzeug headers, and we are breaking its API currently, and should properly use the data structure. I probably wouldn't limit the fix to Set-Cookie.

In itself and with the libraires we have been using, we didn't see the difference in behavior until now (which was kinda bad) but also shows that we shouldn't have an issue the other way around.

[cloutierMat]

Awesome! Thanks for the extra context @bentsku!