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

update expected result for V4 Signed URL to follow changes made in ht… #20

Closed
wants to merge 2 commits into from

Conversation

IlyaFaer
Copy link

…tps://github.com/googleapis/python-storage/pull/48

@IlyaFaer IlyaFaer changed the title change expected result for V4 Signed URL to follow changes made in ht… update expected result for V4 Signed URL to follow changes made in ht… Feb 12, 2020
@IlyaFaer IlyaFaer marked this pull request as ready for review February 12, 2020 14:14
@IlyaFaer
Copy link
Author

IlyaFaer commented Feb 12, 2020

@frankyn, I didn't find test data in url_signed_v4_test_data.json for ordering and encoding tests. I suppose it's not yet generated or something. So, I've inserted them manually to check if tests are working with the new expected results

image

@frankyn
Copy link
Member

frankyn commented Feb 12, 2020

Hi @IlyaFaer, thanks for sending this PR.

I think we have crossed wires here.

SignedHeaders is incorrectly including query parameter. The current expectedUrls are correct and verified with the GCS team.

I can help clarify any questions you may have. I'm closing this PR for now and I appreciate your patience with the confusion.

@frankyn frankyn closed this Feb 12, 2020
@frankyn
Copy link
Member

frankyn commented Feb 12, 2020

Ah whoops, the issue is that dates are formatted correctly in your changes. I misread the url.

     "expectedUrl": "https://storage.googleapis.com/test-bucket/test-object?X-Goog-Algorithm=GOOG4-RSA-SHA256&X-Goog-Credential=test-iam-credentials%40dummy-project-id.iam.gserviceaccount.com%2F20190201%2Fauto%2Fstorage%2Fgoog4_request&X-Goog-Date=20190201T090000Z&X-Goog-Expires=10&X-Goog-SignedHeaders=host&aA0%C3%A9%2F%3D%25-_.~=~._-%25%3D%2F%C3%A90Aa&X-Goog-Signature=221f1905382ce560042b0441e678b6589f4a661fd319f079bde1f7d7ab07e8515334dbabd901c95d3f16a03f389f661ef7de897fdc7cea8914d93ac8638ad56a9dca62ec8983478a9513a702e12dd57182b5b5ee58d7e94dd685f6c2bbaec1ad168294eaf8300cafec7565e1ad99f55b324caa48720d541e1b2be39b10baa7ff39d2cb77efdad91d63fa0c80625234430027077f68f8ad8c258ef8aba93e2a15fb3f74111e9ffab46f481899d1e83db7d84d9b2645975086ba67ce2d9284d50bb2725871d05621a791ee1c9db7db8a52d579191c5f59da6063128effbe0bbc1ae9a573e298e63aa29bbe9bb8dba76a6c98154a9f03f5ce0cb8f176e0ad14acae",
      "expectedUrl": "https://storage.googleapis.com/test-bucket/test-object?X-Goog-Algorithm=GOOG4-RSA-SHA256&X-Goog-Credential=test-iam-credentials%40dummy-project-id.iam.gserviceaccount.com%2F2019-02-%2Fauto%2Fstorage%2Fgoog4_request&X-Goog-Date=2019-02-01T09%3A00%3A00Z&X-Goog-Expires=10&X-Goog-SignedHeaders=host&aA0%C3%A9%2F%3D%25-_.~=~._-%25%3D%2F%C3%A90Aa&X-Goog-Signature=3fbb217b9c0e7f0cf39cc1646d34f44bb4a84bfb8101324851b9580a184129f9bc474023fc95c52d69b28bab246fe76a32d8b33299575e8faf6c0a5eb693a57bff6ecff595cd248aa4485d6ba83a9e46b5903d169c2d86b7a70fe97e66f0db5784d626bf3b3f298320b7faf40be68bb04a48ec187f2ca93e2fd1c18f1d85b355663bcfe2eaf2be8d73108006558419091f6869742f61980f6df5397dc67f375fd1aca0554bb52497b835fa929e6f840f721f102d1789dc0e99ae62a3be78fe05b2d07b9b4818158fb185c94d18ba7ee0e7490d837aee8c811148a802bbe690a6ef72310c59415f1cdb3130fb046b0283419a8262ee2c7cc23312722f5c8a11d5",

Specifically

The following:

test-iam-credentials%40dummy-project-id.iam.gserviceaccount.com%2F2019-02-%2Fauto%2Fstorage%2Fgoog4_request

Should be:

test-iam-credentials%40dummy-project-id.iam.gserviceaccount.com%2F20190201%2Fauto%2Fstorage%2Fgoog4_request

Additionally

The following:

X-Goog-Date=2019-02-01T09%3A00%3A00Z

Should be:

X-Goog-Date=20190201T090000Z

Hope that helps.

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

Successfully merging this pull request may close these issues.

None yet

2 participants