twisted / txaws

Twisted-based Asynchronous Libraries for Amazon Web Services and clouds that support the AWS APIs
MIT License
32 stars 18 forks source link

Fix signing encoding handling. #61

Closed mithrandi closed 7 years ago

mithrandi commented 7 years ago

Fixes #20.

Note that while this fixes some cases that were broken in 0.2.3 as well as 0.3.0 (eg. b'object with spaces'), some of the cases were working in 0.2.3 and are now broken in 0.3.0 (eg. b'object:with:colons'), so this is a regression fix.

codecov[bot] commented 7 years ago

Codecov Report

Merging #61 into master will decrease coverage by 0.4%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #61      +/-   ##
==========================================
- Coverage   96.36%   95.95%   -0.41%     
==========================================
  Files          74       74              
  Lines        8891     8906      +15     
  Branches      627      629       +2     
==========================================
- Hits         8568     8546      -22     
- Misses        201      230      +29     
- Partials      122      130       +8
Impacted Files Coverage Δ
txaws/testing/s3.py 88.88% <100%> (+1.38%) :arrow_up:
txaws/client/base.py 88.25% <100%> (-1.95%) :arrow_down:
txaws/_auth_v4.py 100% <100%> (ø) :arrow_up:
txaws/testing/s3_tests.py 100% <100%> (ø) :arrow_up:
txaws/route53/client.py 87.5% <0%> (-12.5%) :arrow_down:
txaws/testing/integration.py 78.57% <0%> (-7.15%) :arrow_down:
txaws/service.py 87.61% <0%> (-6.67%) :arrow_down:
txaws/client/ssl.py 71.25% <0%> (-3.75%) :arrow_down:
txaws/s3/client.py 94.08% <0%> (-0.81%) :arrow_down:
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 247dda5...b76c611. Read the comment docs.

mithrandi commented 7 years ago

This branch is in the wrong repo so the integration tests were skipped on Travis; however they did pass for me locally.

exarkun commented 7 years ago

See #69 for test results / coverage report including the integration test suite. Results there look good.

mithrandi commented 7 years ago

@exarkun: Okay, addressed; think we're good to merge here (sorry it took so long to get back to this).

exarkun commented 7 years ago

Great! Thanks. Did another thing with #71 to see integration test suite results and they look good. So, approved & merging.