Skip to content
This repository has been archived by the owner on Jun 25, 2021. It is now read-only.

HTTP Headers are now lowercase #71

Closed
artfulrobot opened this issue Feb 4, 2020 · 2 comments
Closed

HTTP Headers are now lowercase #71

artfulrobot opened this issue Feb 4, 2020 · 2 comments

Comments

@artfulrobot
Copy link
Owner

artfulrobot commented Feb 4, 2020

Everybody is receiving this email from GC:

We are writing to inform you of a recent change which may affect your integration with GoCardless. This update is intended for the developer contact for your integration. If you haven’t built your own integration, please forward this onto the engineers who built the integration for you.

This change only affects integrators who are using HTTP/1.1, whose code expects the HTTP response header keys to be in a specific case.

If you are using HTTP/2.0, you can ignore this email.

What has changed?

As part of our routine infrastructure upgrades we have deployed a change that downcases our HTTP response headers for HTTP/1.1 requests. This is in keeping with the HTTP/1.1 specification which states that header keys are case insensitive, and helps align us with security best practices to prevent HTTP request smuggling attacks.

If your application makes HTTP/2 requests:

Your response headers will always have been downcased. This change brings the same behaviour to HTTP/1.1 requests as we have with HTTP/2, so headers such as Content-Type may present as content-type.

For applications using HTTP/1.1:

Please check your integration handles header keys as case-insensitive values. This will make you compatible with future HTTP versions, and ensures your application handles any changes that may occur as other systems across the internet make this change.

Please note that all integrators must treat our HTTP response headers as case-insensitive (i.e. could be sentence case or downcase) while using any version of HTTP.

If you would like to discuss this or require any assistance, please contact us on [email protected].

Thanks,

GoCardless Team

I don't think this is anything to worry about - assuming response headers being case insensitive is handled by guzzle (which is used by GoCardless's own code). The only other place we look at headers is in processing webhooks. This is fixed by 255e1e9

Nb. GC uses guzzle 6.0+ but CiviCRM has already brought in guzzle 6.3+, so I think that should be fine.

GoCardless docs at here and here are still referring to Webhook-Signature but they have confirmed this is wrong and that they will update these

@artfulrobot
Copy link
Owner Author

artfulrobot commented Feb 4, 2020

You can download 1.9.2beta to test this
https://github.com/artfulrobot/uk.artfulrobot.civicrm.gocardless/releases/tag/1.9.2beta
(note that the release includes some other changes, too)

Should you need to downgrade, you can just replace the code with the 1.9.1 version again as there are no one-way changes made oops, there are one-way changes made. However, these are unlikely to affect you, and you would still be able to downgrade.

@artfulrobot
Copy link
Owner Author

Contrary to the engineer's response on twitter, I go this from GoCardless today:

Webhook headers will not be affected by this change.

It's only the http headers on our API responses (i.e. when you send us a request) that are affected.

Which means that the HTTP headers thing is nothing to worry about - you don't need to upgrade to 1.9.2beta2, although if you have, there's no need to downgrade either.

I'll keep this commit in, as it does no harm and should they ever decide to send "webhook-signature" or "WeBHoOk-SiGNatURe" it will still work!

Closing this issue.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

1 participant