search

artfulrobot / uk.artfulrobot.civicrm.gocardless

A CiviCRM extension providing GoCardless integration to handle UK Direct Debits.
GNU Affero General Public License v3.0
5 stars 18 forks source link

HTTP Headers are now lowercase #71

Closed artfulrobot closed 4 years ago

artfulrobot commented 4 years ago

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 help@gocardless.com.

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 https://github.com/artfulrobot/uk.artfulrobot.civicrm.gocardless/commit/255e1e919042e727ac2e53285077423c1d7a2d72

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 commented 4 years ago

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 commented 4 years ago

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.