From 84288d08ab49387ac4211631b1f4122fddd0547b Mon Sep 17 00:00:00 2001 From: Nathaniel Case Date: Fri, 14 May 2021 09:24:05 -0400 Subject: [PATCH 1/6] Report sftp_error when write fails --- src/pylibsshext/sftp.pyx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pylibsshext/sftp.pyx b/src/pylibsshext/sftp.pyx index 01d15805d..58128d8a0 100644 --- a/src/pylibsshext/sftp.pyx +++ b/src/pylibsshext/sftp.pyx @@ -70,7 +70,7 @@ cdef class SFTP: written = sftp.sftp_write(rf, PyBytes_AS_STRING(buffer), length) if written != length: sftp.sftp_close(rf) - raise LibsshSFTPException("Writing to remote file [%s] failed" % remote_file) + raise LibsshSFTPException("Writing to remote file [%s] failed with error [%s]" % (remote_file, MSG_MAP.get(self._get_sftp_error_str()))) buffer = f.read(1024) sftp.sftp_close(rf) From 880cef8d37b6b4029b500f598a200ddc4b4419cb Mon Sep 17 00:00:00 2001 From: Nathaniel Case Date: Fri, 14 May 2021 10:12:56 -0400 Subject: [PATCH 2/6] Let's look at ssh_error, too --- src/pylibsshext/sftp.pyx | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/pylibsshext/sftp.pyx b/src/pylibsshext/sftp.pyx index 58128d8a0..65ef59ec2 100644 --- a/src/pylibsshext/sftp.pyx +++ b/src/pylibsshext/sftp.pyx @@ -70,7 +70,13 @@ cdef class SFTP: written = sftp.sftp_write(rf, PyBytes_AS_STRING(buffer), length) if written != length: sftp.sftp_close(rf) - raise LibsshSFTPException("Writing to remote file [%s] failed with error [%s]" % (remote_file, MSG_MAP.get(self._get_sftp_error_str()))) + raise LibsshSFTPException( + "Writing to remote file [%s] failed with error [%s: %s]" % ( + remote_file, + MSG_MAP.get(self._get_sftp_error_str()), + self.session._get_session_error_str(), + ) + ) buffer = f.read(1024) sftp.sftp_close(rf) From d6dba2aeda0fa5e15fb763ad7dabe1f7fed5b2fc Mon Sep 17 00:00:00 2001 From: Nathaniel Case Date: Thu, 3 Jun 2021 10:55:14 -0400 Subject: [PATCH 3/6] Add changelog --- docs/changelog-fragments/216.bugfix.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 docs/changelog-fragments/216.bugfix.rst diff --git a/docs/changelog-fragments/216.bugfix.rst b/docs/changelog-fragments/216.bugfix.rst new file mode 100644 index 000000000..7ece219ed --- /dev/null +++ b/docs/changelog-fragments/216.bugfix.rst @@ -0,0 +1 @@ +Added additional error reporting to SFTP write errors -- :user: Qalthos From 1d6956698b9481a6c3b3db4a2fbdc04781e6ffcf Mon Sep 17 00:00:00 2001 From: Nathaniel Case Date: Mon, 7 Jun 2021 12:49:22 -0400 Subject: [PATCH 4/6] sftp_get: write bytes directly, don't expect them to be unicode --- docs/changelog-fragments/216.bugfix.2.rst | 1 + docs/changelog-fragments/216.bugfix.rst | 2 +- src/pylibsshext/sftp.pyx | 8 ++++---- 3 files changed, 6 insertions(+), 5 deletions(-) create mode 100644 docs/changelog-fragments/216.bugfix.2.rst diff --git a/docs/changelog-fragments/216.bugfix.2.rst b/docs/changelog-fragments/216.bugfix.2.rst new file mode 100644 index 000000000..9e094435a --- /dev/null +++ b/docs/changelog-fragments/216.bugfix.2.rst @@ -0,0 +1 @@ +Changed ``sftp.sftp_get`` to write files as bytes rather than assuming files are valid UTF8 -- by :user:`Qalthos` diff --git a/docs/changelog-fragments/216.bugfix.rst b/docs/changelog-fragments/216.bugfix.rst index 7ece219ed..67e03f3b9 100644 --- a/docs/changelog-fragments/216.bugfix.rst +++ b/docs/changelog-fragments/216.bugfix.rst @@ -1 +1 @@ -Added additional error reporting to SFTP write errors -- :user: Qalthos +Added additional error reporting to SFTP write errors -- by :user:`Qalthos` diff --git a/src/pylibsshext/sftp.pyx b/src/pylibsshext/sftp.pyx index 65ef59ec2..53688561e 100644 --- a/src/pylibsshext/sftp.pyx +++ b/src/pylibsshext/sftp.pyx @@ -101,14 +101,14 @@ cdef class SFTP: raise LibsshSFTPException("Reading data from remote file [%s] failed with error [%s]" % (remote_file, MSG_MAP.get(self._get_sftp_error_str()))) - with open(local_file, 'w+') as f: - bytes_wrote = f.write(read_buffer[:file_data].decode('utf-8')) - if bytes_wrote and file_data != bytes_wrote: + with open(local_file, 'wb+') as f: + bytes_written = f.write(read_buffer[:file_data]) + if bytes_written and file_data != bytes_written: sftp.sftp_close(rf) raise LibsshSFTPException("Number of bytes [%s] read from remote file [%s]" " does not match number of bytes [%s] written to local file [%s]" " due to error [%s]" - % (file_data, remote_file, bytes_wrote, local_file, MSG_MAP.get(self._get_sftp_error_str()))) + % (file_data, remote_file, bytes_written, local_file, MSG_MAP.get(self._get_sftp_error_str()))) sftp.sftp_close(rf) def close(self): From e1a5dfbd5c8b127e81c3b613ce03973272fad7dd Mon Sep 17 00:00:00 2001 From: Nathaniel Case Date: Tue, 8 Jun 2021 11:26:33 -0400 Subject: [PATCH 5/6] Change _get_sftp_error_str to actually return strs --- src/pylibsshext/sftp.pyx | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/pylibsshext/sftp.pyx b/src/pylibsshext/sftp.pyx index 53688561e..6331c529d 100644 --- a/src/pylibsshext/sftp.pyx +++ b/src/pylibsshext/sftp.pyx @@ -62,7 +62,7 @@ cdef class SFTP: rf = sftp.sftp_open(self._libssh_sftp_session, remote_file_b, O_WRONLY | O_CREAT | O_TRUNC, sftp.S_IRWXU) if rf is NULL: - raise LibsshSFTPException("Opening remote file [%s] for write failed with error [%s]" % (remote_file, MSG_MAP.get(self._get_sftp_error_str()))) + raise LibsshSFTPException("Opening remote file [%s] for write failed with error [%s]" % (remote_file, self._get_sftp_error_str())) buffer = f.read(1024) while buffer != b"": @@ -71,10 +71,9 @@ cdef class SFTP: if written != length: sftp.sftp_close(rf) raise LibsshSFTPException( - "Writing to remote file [%s] failed with error [%s: %s]" % ( + "Writing to remote file [%s] failed with error [%s]" % ( remote_file, - MSG_MAP.get(self._get_sftp_error_str()), - self.session._get_session_error_str(), + self._get_sftp_error_str(), ) ) buffer = f.read(1024) @@ -90,7 +89,7 @@ cdef class SFTP: rf = sftp.sftp_open(self._libssh_sftp_session, remote_file_b, O_RDONLY, sftp.S_IRWXU) if rf is NULL: - raise LibsshSFTPException("Opening remote file [%s] for read failed with error [%s]" % (remote_file, MSG_MAP.get(self._get_sftp_error_str()))) + raise LibsshSFTPException("Opening remote file [%s] for read failed with error [%s]" % (remote_file, self._get_sftp_error_str())) while True: file_data = sftp.sftp_read(rf, read_buffer, sizeof(char) * 1024) @@ -99,7 +98,7 @@ cdef class SFTP: elif file_data < 0: sftp.sftp_close(rf) raise LibsshSFTPException("Reading data from remote file [%s] failed with error [%s]" - % (remote_file, MSG_MAP.get(self._get_sftp_error_str()))) + % (remote_file, self._get_sftp_error_str())) with open(local_file, 'wb+') as f: bytes_written = f.write(read_buffer[:file_data]) @@ -108,7 +107,7 @@ cdef class SFTP: raise LibsshSFTPException("Number of bytes [%s] read from remote file [%s]" " does not match number of bytes [%s] written to local file [%s]" " due to error [%s]" - % (file_data, remote_file, bytes_written, local_file, MSG_MAP.get(self._get_sftp_error_str()))) + % (file_data, remote_file, bytes_written, local_file, self._get_sftp_error_str())) sftp.sftp_close(rf) def close(self): @@ -117,4 +116,7 @@ cdef class SFTP: self._libssh_sftp_session = NULL def _get_sftp_error_str(self): - return sftp.sftp_get_error(self._libssh_sftp_session) + error = sftp.sftp_get_error(self._libssh_sftp_session) + if error in MSG_MAP and error != sftp.SSH_FX_FAILURE: + return MSG_MAP[error] + return "Generic failure: %s" % self.session._get_session_error_str() From ea66107216d3c64b21a52aa10e8b60e574b78c8c Mon Sep 17 00:00:00 2001 From: Nathaniel Case Date: Wed, 20 Oct 2021 15:44:00 -0400 Subject: [PATCH 6/6] Rework changelog fragments --- docs/changelog-fragments/216.bugfix.2.rst | 1 - docs/changelog-fragments/216.bugfix.rst | 2 +- docs/changelog-fragments/216.misc.rst | 1 + 3 files changed, 2 insertions(+), 2 deletions(-) delete mode 100644 docs/changelog-fragments/216.bugfix.2.rst create mode 100644 docs/changelog-fragments/216.misc.rst diff --git a/docs/changelog-fragments/216.bugfix.2.rst b/docs/changelog-fragments/216.bugfix.2.rst deleted file mode 100644 index 9e094435a..000000000 --- a/docs/changelog-fragments/216.bugfix.2.rst +++ /dev/null @@ -1 +0,0 @@ -Changed ``sftp.sftp_get`` to write files as bytes rather than assuming files are valid UTF8 -- by :user:`Qalthos` diff --git a/docs/changelog-fragments/216.bugfix.rst b/docs/changelog-fragments/216.bugfix.rst index 67e03f3b9..9e094435a 100644 --- a/docs/changelog-fragments/216.bugfix.rst +++ b/docs/changelog-fragments/216.bugfix.rst @@ -1 +1 @@ -Added additional error reporting to SFTP write errors -- by :user:`Qalthos` +Changed ``sftp.sftp_get`` to write files as bytes rather than assuming files are valid UTF8 -- by :user:`Qalthos` diff --git a/docs/changelog-fragments/216.misc.rst b/docs/changelog-fragments/216.misc.rst new file mode 100644 index 000000000..e1fc57896 --- /dev/null +++ b/docs/changelog-fragments/216.misc.rst @@ -0,0 +1 @@ +Added additional details when SFTP write errors are raised -- by :user:`Qalthos`