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

Remove BytesWarnings in client.py by putting u() around every hexlify, other BytesWarnings fixes #1661

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

aggieNick02
Copy link

I run python in an environment where BytesWarnings are enabled and promoted to errors. This was really helpful during py2->3 transitioning, and still helps prevent misuse of bytes as strings.

I'd like to keep doing this while using Paramiko, but I hit BytesWarnings when doing so. I looked at related issues for paramiko, and they've previously been fixed by putting a u() around the offending bytes. This is appropriate for calls to hexlify that are then passed to format(), since hexlify returns the bytes for the ASCII-encoded hex rep of the input.

This is what was done for the same kind of issue previously: #1074

I also fixed the same kind of issue, in auth_client.py, reported at:
#1593

This should also close the issue 728 linked below. The issue requested a string-ish hex-digest, which would be more dramatic and risky for a wholesale transition, since there could be clients that still need the bytes-ish one. But the u() around the hexlifys going to format safely resolves being able to run with -bb.:
#726

I went ahead and found the rest of the uses of hexlify that were exclusively passed to format/print, and fixed similarly. I left other uses of hexllify alone, as there are definitely still places, e.g. tests, that fully expect bytes back.

@aggieNick02
Copy link
Author

Sorry, didn't realize there were whitespace checks. Do you want me to fix and supply a new PR?

ploxiln added a commit to ploxiln/paramiko-ng that referenced this pull request Apr 11, 2020
Transport._log() does not take msg list, it takes format string

SFTPClient/SFTPServer _log() method does not take msg list
SFTPBase gets new _loglist() method for the rare list case

Transport and SFTP* use proper logging format strings
and separate arguments

SFTPClient no longer needs %% escaping of log messages,
now that it logs correctly

SSHClient and AuthHandler always log with single msg string,
so pass to Transport._log() as "%s", msg

get rid of util.tb_strings(), instead _log() with exc_info=True
so that lines of traceback are not separate log messages
(inspired by paramiko#406)

binascii.hexlify() returns bytes, decode to string for logging
(inspired by paramiko#1661)
ploxiln added a commit to ploxiln/paramiko-ng that referenced this pull request Apr 11, 2020
Transport._log() does not take msg list, it takes format string

SFTPClient/SFTPServer _log() method does not take msg list
SFTPBase gets new _loglist() method for the rare list case

Transport and SFTP* use proper logging format strings
and separate arguments

SFTPClient no longer needs %% escaping of log messages,
now that it logs correctly

SSHClient and AuthHandler always log with single msg string,
so pass to Transport._log() as "%s", msg

get rid of util.tb_strings(), instead _log() with exc_info=True
so that lines of traceback are not separate log messages
(inspired by paramiko#406)

binascii.hexlify() returns bytes, decode to string for logging
(inspired by paramiko#1661)
ploxiln added a commit to ploxiln/paramiko-ng that referenced this pull request Apr 17, 2020
Transport._log() does not take msg list, it takes format string

SFTPClient/SFTPServer _log() method does not take msg list
SFTPBase gets new _loglist() method for the rare list case

Transport and SFTP* use proper logging format strings
and separate arguments

SFTPClient no longer needs %% escaping of log messages,
now that it logs correctly

SSHClient and AuthHandler always log with single msg string,
so pass to Transport._log() as "%s", msg

get rid of util.tb_strings(), instead _log() with exc_info=True
so that lines of traceback are not separate log messages
(inspired by paramiko#406)

binascii.hexlify() returns bytes, decode to string for logging
(inspired by paramiko#1661)
ploxiln added a commit to ploxiln/paramiko-ng that referenced this pull request Apr 20, 2020
Transport._log() does not take msg list, it takes format string

SFTPClient/SFTPServer _log() method does not take msg list
SFTPBase gets new _loglist() method for the rare list case

Transport and SFTP* use proper logging format strings
and separate arguments

SFTPClient no longer needs %% escaping of log messages,
now that it logs correctly

SSHClient and AuthHandler always log with single msg string,
so pass to Transport._log() as "%s", msg

get rid of util.tb_strings(), instead _log() with exc_info=True
so that lines of traceback are not separate log messages
(inspired by paramiko#406)

binascii.hexlify() returns bytes, decode to string for logging
(inspired by paramiko#1661)
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.

1 participant