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 #664 #665

Merged
merged 4 commits into from
Jul 19, 2017
Merged

fix #664 #665

merged 4 commits into from
Jul 19, 2017

Conversation

reaperhulk
Copy link
Member

No description provided.

bytes and strings are different things.
@alex
Copy link
Member

alex commented Jul 19, 2017

a) Why don't the tests catch this?
b) If these values are going to be unicode, please make _CRYPTOGRAPHY_MANYLINUX1_CA_DIR unicode.

@alex alex added this to the 17.2.0 milestone Jul 19, 2017
@reaperhulk
Copy link
Member Author

reaperhulk commented Jul 19, 2017

Tests didn't catch this because to have it take the fallback path we fetched the values from cffi and then monkeypatched our constants (/~https://github.com/pyca/pyopenssl/blob/master/tests/test_ssl.py#L1114-L1142). So the comparison succeed since they were both bytes.

I think a better fix here is just to declare these as byte strings and not unicode. They're not really paths to us, just sentinel values.

@alex
Copy link
Member

alex commented Jul 19, 2017

That's also fine with me.

@codecov
Copy link

codecov bot commented Jul 19, 2017

Codecov Report

Merging #665 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #665   +/-   ##
=======================================
  Coverage   96.94%   96.94%           
=======================================
  Files          18       18           
  Lines        5737     5737           
  Branches      401      401           
=======================================
  Hits         5562     5562           
  Misses        117      117           
  Partials       58       58
Impacted Files Coverage Δ
src/OpenSSL/SSL.py 94.88% <100%> (ø) ⬆️

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 7f5610c...4041f61. Read the comment docs.

@alex
Copy link
Member

alex commented Jul 19, 2017

trailing whitespace makes the flake8 fail

@reaperhulk
Copy link
Member Author

Why can't GitHub's editor strip trailing whitespace like my vim config does.

@alex
Copy link
Member

alex commented Jul 19, 2017

Send a feature request!

@alex alex merged commit a92a1a7 into master Jul 19, 2017
@alex alex deleted the reaperhulk-patch-1 branch July 19, 2017 13:53
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants