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 two issues with writing private key with passphrase #772

Closed
wants to merge 3 commits into from

Conversation

@cjh1
Copy link

@cjh1 cjh1 commented Jul 8, 2016

See #568

@cjh1 cjh1 changed the title Fix two issues with write private key with passphrase Fix two issues with writing private key with passphrase Jul 8, 2016
@coveralls
Copy link

@coveralls coveralls commented Jul 8, 2016

Coverage Status

Coverage decreased (-0.8%) to 71.764% when pulling 4992d18 on cjh1:write-key-fixes into 0ae019c on paramiko:master.

@cjh1 cjh1 force-pushed the cjh1:write-key-fixes branch from 4992d18 to 998f223 Jul 8, 2016
@coveralls
Copy link

@coveralls coveralls commented Jul 8, 2016

Coverage Status

Coverage increased (+0.1%) to 72.659% when pulling 998f223 on cjh1:write-key-fixes into 0ae019c on paramiko:master.

@@ -342,7 +342,7 @@ def _write_private_key(self, f, key, format, password=None):
if password is None:
encryption = serialization.NoEncryption()
else:
encryption = serialization.BestEncryption(password)
encryption = serialization.BestAvailableEncryption(password.encode())

This comment has been minimized.

@alexpilotti

alexpilotti Aug 23, 2016

The method does not specify if the "password" parameter is bytes or string. If it's not documented elsewhere, wouldn't it be better to expect bytes in input? This affects python3. As an alternative, we could accept both types and call encode() only if the type is string.

This comment has been minimized.

@cjh1

cjh1 Aug 23, 2016
Author

All the public methods in this class that take a password are expecting strings, so I think this password parameter should also be a string. Should I add a doc string to indicate this?

@alexpilotti
Copy link

@alexpilotti alexpilotti commented Aug 23, 2016

This fixes issue #741

@bitprophet
Copy link
Member

@bitprophet bitprophet commented Jun 7, 2017

Rolling into #741.

@bitprophet bitprophet closed this Jun 7, 2017
@cjh1
Copy link
Author

@cjh1 cjh1 commented Jun 7, 2017

So have these fixes been merged?

@ploxiln
Copy link
Contributor

@ploxiln ploxiln commented Jun 7, 2017

yes, they'll be in the next 2.y.z releases

@bitprophet
Copy link
Member

@bitprophet bitprophet commented Jun 7, 2017

Been laid low today but yea I'm planning to release all this stuff as soon as I can. May toss in some low hanging fruit from the bugfix milestone too, but there's already so much ready to go out I don't absolutely have to.

@cjh1
Copy link
Author

@cjh1 cjh1 commented Jun 8, 2017

Excellent! Many Thanks

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

Successfully merging this pull request may close these issues.

None yet

5 participants