Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: allow space between checksums in header #170

Merged
merged 2 commits into from Sep 10, 2020

Conversation

mcsimps2
Copy link
Contributor

@mcsimps2 mcsimps2 commented Sep 8, 2020

The current checksum logic only allowed checksum headers in the format crc32c=hash1,md5=hash2, but GCP may actually return them with a space in-between as crc32c=hash1, md5=hash2.

This means that checksums are not being verified correctly for any header response like the latter.

If you want an actual example of a response from GCP that demonstrates this, here's one:

{
   "X-GUploader-UploadID":"REDACTED",
   "Expires":"Tue, 08 Sep 2020 21:27:47 GMT",
   "Date":"Tue, 08 Sep 2020 21:27:47 GMT",
   "Cache-Control":"private, max-age=0",
   "Last-Modified":"Tue, 08 Sep 2020 21:27:47 GMT",
   "ETag":"REDACTED",
   "x-goog-generation": "REDACTED",
   "x-goog-metageneration":"1",
   "x-goog-stored-content-encoding":"identity",
   "x-goog-stored-content-length":"1043237",
   "Content-Type":"application/octet-stream",
   "x-goog-hash":"crc32c=t6zgHw==, md5=zH3E9XwJPGGra4vnT+aamQ==",
   "x-goog-storage-class":"STANDARD",
   "Accept-Ranges":"bytes",
   "Content-Length":"1043237",
   "Server":"UploadServer"
}

Resolves #169

@google-cla
Copy link

google-cla bot commented Sep 8, 2020

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@google-cla google-cla bot added the cla: no This human has *not* signed the Contributor License Agreement. label Sep 8, 2020
@google-cla
Copy link

google-cla bot commented Sep 8, 2020

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

The current checksum logic only allowed checksum
 headers in the format "crc32c=hash1,md5=hash2",
 but GCP may actually return them with a space
 in-between as "crc32c=hash1, md5=hash2".

Resolves googleapis#169
@google-cla google-cla bot added cla: yes This human has signed the Contributor License Agreement. and removed cla: no This human has *not* signed the Contributor License Agreement. labels Sep 8, 2020
@crwilcox
Copy link
Collaborator

Hi @mcsimps2 thank you for the contribution. Could I bother you to make a corresponding issue for this?

@mcsimps2
Copy link
Contributor Author

@crwilcox It's mentioned in the summary at the bottom - it resolves #169. I should've made it more clear that was the issue of concern, sorry about that!

@crwilcox
Copy link
Collaborator

@mcsimps2 Cool, I just missed it :)

@crwilcox crwilcox added the kokoro:run Add this label to force Kokoro to re-run the tests. label Sep 10, 2020
@yoshi-kokoro yoshi-kokoro removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Sep 10, 2020
@crwilcox
Copy link
Collaborator

Tests are reporting failed for linting, which is not your pr. I can create a separate PR and clean that up.

@crwilcox crwilcox added the kokoro:run Add this label to force Kokoro to re-run the tests. label Sep 10, 2020
@yoshi-kokoro yoshi-kokoro removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Sep 10, 2020
@crwilcox crwilcox merged commit 224fc98 into googleapis:master Sep 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow space in-between checksums in response headers
3 participants