-31: SFTP Protocol Error when transferring file via SFTP Link

[pingu]

GNU Midnight Commander 4.8.13
Built with GLib 2.42.1
Using the S-Lang library with terminfo database
With builtin Editor
With subshell support as default
With support for background operations
With mouse support on xterm and Linux console
With support for X11 events
With internationalization support
With multiple codepages support
Virtual File Systems: cpiofs, tarfs, sfs, extfs, ext2undelfs, ftpfs, sftpfs, fish
Data types: char: 8; int: 32; long: 32; void *: 32; size_t: 32; off_t: 64;

---

Package: libssh2-1
Source: libssh2
Version: 1.4.3-4

---

Local version string SSH-2.0-OpenSSH_6.7p1 Debian-3
Remote protocol version 2.0, remote software version dropbear_0.52

---

Seems that after OpenSSH Server update after infamous Heartblead bug, Im allways getting these errors when copying / moving files within SFTP built in mc:

COPY/MOVE FILE TO REMOTE: "-31: SFTP Protocol Error"
REMOVE REMOTE FILE/DIR: "-31: Failed opening remote file"

Although after I press ENTER - operation continues successfully, however when transfering lots of selected files I have to acknowledge (press ENTER) on every file operation, so batch copy / move / remove operations are inefficient.

Doing verbose CLI sftp connection to the same server, succeeds without errors.
Also to note that to speed up transfers in ssh config file I prioritised blowfish protocol, what is honoured by sftp transfers, but NOT with mc sftp, as with sftp CLI (specifying blowfish) I get allmost twice the speed, but mc sftp transfer speeds are similar to the ones when I force AES encryption on CLI sftp.

[jkeil]

I did a git bisect and came across [512ad7d96250ea5a2ab2197e0b8a02af29d419d1] which was a re-factoring of the error handling.
Previously ([512ad7d96250ea5a2ab2197e0b8a02af29d419d1]) the error would just be swallowed. Now, after the re-factoring it is properly propagated and displayed.

The error originates from sftpfs_opendir in src/vfs/sftpfs/dir.c.

[and]

"-31: SFTP Protocol Error" message can be triggered when entering a remote directory with a symlink pointed to file which located at non readable directory.

/testfile -> /non-accessible-directory/testfile

mc use libssh2_sftp_stat_ex with LIBSSH2_SFTP_LSTAT type, remote lstat() failed
and currently mc not handled this event correctly.

first we need to pimp up sftpfs_ssherror_to_gliberror() to get knowledge about calling libssh2 function and libssh2_sftp_last_error() error code when LIBSSH2_ERROR_SFTP_PROTOCOL (aka -31) occurs.

[and]

first patch incorporate sftp session error hint

[drankinatty]

Just wanted to bump this ticket to say this is still a problem in:
Version : 4.8.18-1
Build Date : Fri 07 Oct 2016 10:34:16 AM CDT (Archlinux)

Upon sftp to remote site, copying file from remote back to originating machine, the following error is displayed:

"Failed opening remote file (-31)"

Upon acknowledging the error by hitting return, the copy completes successfully despite the error.

[sand]

Any progress?

The version in debian/sid also segv after a while (3:4.8.18-1).

I'm trying to copy a bunch of files over a raspbian/jessie, the path I'm copying into doesn't have any symlink and any user can read and traverse every directory of the destination path.

I tried mc from sources (4.8.18-163-g67b3d6495/glib:2.50.2) without and with both the patches above, it doesn't segv but the error -31 is still present.

Ignoring the error I've copied about 5600 files (49M) without errors (md5 checked), now if I try to overwrite just one it segv, the terminal show a ridiculous alloc failed in glib/gmem.c

(mc:31619): GLib-ERROR **: /build/glib2.0-m2w47E/glib2.0-2.50.2/./glib/gmem.c:100: failed to allocate 94337158982256 bytes

0x00007f52702f6261 in ?? () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
(gdb) bt
#0  0x00007f52702f6261 in ?? () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#1  0x00007f52702f72b7 in g_log_default_handler () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#2  0x00007f52702f75c4 in g_logv () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#3  0x00007f52702f77cf in g_log () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#4  0x00007f52702f5e34 in g_malloc () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#5  0x0000559568856781 in copy_file_file (tctx=tctx@entry=0x55956a836730, ctx=ctx@entry=0x55956a8367c0, 
    src_path=0x55956a8366e0 "/home/alex/tmp/yii2-basic/basic/.gitignore",
    dst_path=dst_path@entry=0x55956a810e70 "/sftp://pi@firefly/var/www/vhosts/yii2bm/.gitignore") at ../../../src/filemanager/file.c:1808
#6  0x0000559568859338 in panel_operate (source_panel=0x55956a80f760, operation=operation@entry=OP_COPY, force_single=force_single@entry=0)
    at ../../../src/filemanager/file.c:2909
#7  0x0000559568850c3c in copy_cmd () at ../../../src/filemanager/cmd.c:802
#8  0x00005595687f7425 in midnight_execute_cmd (sender=0x55956a81e800, command=22) at ../../../src/filemanager/midnight.c:1142
#9  0x00005595687dc0f0 in buttonbar_callback (w=0x55956a81e800, sender=<optimized out>, msg=<optimized out>, parm=<optimized out>, 
    data=<optimized out>) at ../../../lib/widget/buttonbar.c:171
#10 0x00005595687e047a in send_message (data=0x0, parm=1005, msg=MSG_HOTKEY, sender=0x0, w=<optimized out>)
    at ../../../lib/widget/widget-common.h:210
#11 dlg_try_hotkey (h=0x55956a819a10, h=0x55956a819a10, d_key=1005) at ../../../lib/widget/dialog.c:434
#12 dlg_key_event (d_key=1005, h=0x55956a819a10) at ../../../lib/widget/dialog.c:479
#13 dlg_process_event (h=0x55956a819a10, key=1005, event=<optimized out>) at ../../../lib/widget/dialog.c:1165
#14 0x00005595687e07d0 in frontend_dlg_run (h=0x55956a819a10) at ../../../lib/widget/dialog.c:541
#15 dlg_run (h=0x55956a819a10) at ../../../lib/widget/dialog.c:1196
#16 0x00005595687f8686 in create_panels_and_run_mc () at ../../../src/filemanager/midnight.c:954
#17 do_nc () at ../../../src/filemanager/midnight.c:1770
#18 0x00005595687d4e9e in main (argc=<optimized out>, argv=<optimized out>) at ../../src/main.c:407

(I'm not sure this is the same issue)

Please let me know if I can help in some way (trying patches)

[zaytsev]

From looking at the code, it seems that the segfault is a regression caused by

18f6b850054ab0b12a17f786191d345a1742cb48

and I think that the reason why it segfaults now is is because sftpfs_{l,f}stat functions do not set st_blksize stuff.

Here is an untested patch which should remove the segfault if my guess is correct:

diff --git a/src/vfs/sftpfs/internal.c b/src/vfs/sftpfs/internal.c
index 5377b66..2301ad5 100644
--- a/src/vfs/sftpfs/internal.c
+++ b/src/vfs/sftpfs/internal.c
@@ -30,6 +30,8 @@
 #include "lib/global.h"
 #include "lib/util.h"
 
+#include "src/filemanager/ioblksize.h"          /* IO_BUFSIZE */
+
 #include "internal.h"
 
 /*** global variables ****************************************************************************/
@@ -193,6 +195,9 @@ sftpfs_lstat (const vfs_path_t * vpath, struct stat *buf, GError ** mcerror)
     if ((attrs.flags & LIBSSH2_SFTP_ATTR_PERMISSIONS) != 0)
         buf->st_mode = attrs.permissions;
 
+    buf->st_blksize = IO_BUFSIZE;
+    buf->st_blocks = 1 + ((buf->st_size - 1) / buf->st_blksize);
+
     return 0;
 }
 
@@ -272,6 +277,9 @@ sftpfs_stat (const vfs_path_t * vpath, struct stat *buf, GError ** mcerror)
     if ((attrs.flags & LIBSSH2_SFTP_ATTR_PERMISSIONS) != 0)
         buf->st_mode = attrs.permissions;
 
+    buf->st_blksize = IO_BUFSIZE;
+    buf->st_blocks = 1 + ((buf->st_size - 1) / buf->st_blksize);
+
     return 0;
 }
 

I hope it does compile.

[sand]

Thanks zaytsev, sorry for the delay didn't get any mail, it does compile and it does fix the overwrite segv!

About the error -31, I see "protocol error" 2 and 4, usually start with 2 then alternate with some 4 (2 and 4 should be "err").

If there's anything else I can do just let me know (strace? ltrace? enable/insert some trace in mc/ssl? enable debug on server side?)

[zaytsev]

I've created a new ticket for the segfault: #3749 .

Re. error -31, sorry, I don't even use sftp and I don't know if and when I will get time to look into it if at all. Too bad and's patches apparently aren't enough to fix the problem :-/

[sand]

I'm not very confident with ssl and this is the first time I see mc sources but, a breakpoint on the error message show bt like this:

#0  sftpfs_ssherror_to_gliberror (super_data=super_data@entry=0x55a4a0fdf510, libssh_errno=-31, mcerror=mcerror@entry=0x7ffc38fdaac8)
    at ../../../../src/vfs/sftpfs/internal.c:63
#1  0x000055a4a062ec9e in sftpfs_stat (vpath=<optimized out>, buf=0x7ffc38fdac90, mcerror=mcerror@entry=0x7ffc38fdaac8)
    at ../../../../src/vfs/sftpfs/internal.c:265
#2  0x000055a4a06222bc in sftpfs_cb_stat (vpath=<optimized out>, buf=<optimized out>) at ../../../../src/vfs/sftpfs/vfs_class.c:277
#3  0x000055a4a05ae4d9 in mc_stat (vpath=vpath@entry=0x55a4a0fb7910, buf=buf@entry=0x7ffc38fdac90) at ../../../lib/vfs/interface.c:571
#4  0x000055a4a0615b30 in copy_file_file (tctx=tctx@entry=0x55a4a0fdf000, ctx=ctx@entry=0x55a4a0fdb600, 

If I understand correctly the error is raised within internal.c:sftpfs_stat() by libssh2_sftp_stat_ex

        res =
            libssh2_sftp_stat_ex (super_data->sftp_session,
                                  fixfname, sftpfs_filename_buffer->len, LIBSSH2_SFTP_STAT, &attrs);
        if (res >= 0)
            break;

        if (res == LIBSSH2_ERROR_SFTP_PROTOCOL &&
            libssh2_sftp_last_error(super_data->sftp_session)  == LIBSSH2_FX_PERMISSION_DENIED)
            return EACCES;

        if (res != LIBSSH2_ERROR_EAGAIN)
        {
            sftpfs_ssherror_to_gliberror (super_data, res, mcerror);
            return -1;
        }

libssh2_sftp_stat_ex return unexpected/unhandled -2 and -4 (res is shown as err), which in my /usr/include/libssh2.h are:

#define LIBSSH2_ERROR_BANNER_RECV               -2
#define LIBSSH2_ERROR_INVALID_MAC               -4

But I don't know ssl enough to understand what is wrong, maybe it require some option to ignore certificates like wget?

[zaytsev]

I'm not sure you are looking at the right errors. You should put a break on sftpfs_ssherror_to_gliberror and check the values of res and libssh2_sftp_last_error(super_data->sftp_session).

You said earlier that you get a message that res == -31, so this can't be caused by res == -2 or -4. To decode protocol errors you need to look at the LIBSSH2_FX_* constants.

[sand]

Sorry, I got the error thing a bit wrong.

libssh2_sftp_stat_ex() is a two line function which call sftp_stat(), this may exit with error -31 (ftp), 2 and 4 are errors from the server.

    if (data[0] == SSH_FXP_STATUS) {
        int retcode;

        retcode = _libssh2_ntohu32(data + 5);
        LIBSSH2_FREE(session, data);
        if (retcode == LIBSSH2_FX_OK) {
            return 0;
        } else {
            sftp->last_errno = retcode;
            return _libssh2_error(session, LIBSSH2_ERROR_SFTP_PROTOCOL,
                                  "SFTP Protocol Error");
        }
    }

I tried to enable-debug in libssh2 in order to get ssh2_strace() working but got into more trouble, time to give up yet, maybe I'll try later with pbuilder.

[zaytsev]

In as far as LIBSSH2_FX_NO_SUCH_FILE is concerned, I think the reason why this happens is that the copy function tries to stat destination file to make sure it's not overwriting anything, but the stat function shows protocol error, instead of correctly propagating the status.

Untested patch to check this assumption:

diff --git a/src/vfs/sftpfs/internal.c b/src/vfs/sftpfs/internal.c
index 5377b66..4d388fe 100644
--- a/src/vfs/sftpfs/internal.c
+++ b/src/vfs/sftpfs/internal.c
@@ -241,6 +241,10 @@ sftpfs_stat (const vfs_path_t * vpath, struct stat *buf, GError ** mcerror)
         if (res >= 0)
             break;
 
+        if (res == LIBSSH2_ERROR_SFTP_PROTOCOL &&
+            libssh2_sftp_last_error(super_data->sftp_session) == LIBSSH2_FX_NO_SUCH_FILE)
+            return ENOENT;
+
         if (res != LIBSSH2_ERROR_EAGAIN)
         {
             sftpfs_ssherror_to_gliberror (super_data, res, mcerror);

To find out why LIBSSH2_FX_FAILURE happens try to find out how to reliably reproduce it and pay attention to call arguments while debugging.

[sand]

I must have too much changes, the patch applied but in the wrong function.

There are 4 call to libssh2_sftp_stat_ex in internal.c but you're right the only one failing is in sftpfs_stat, and with your patch -31:2 is gone.

Let see if a backtrace for the -31:4 can help.

[sand]

I tried some copy and every backtrace looks almost the same, the failure seems to be from sftpfs_chmod() (on created directory maybe?)

#0  sftpfs_ssherror_to_gliberror (super_data=super_data@entry=0x55765957cb20, libssh_errno=libssh_errno@entry=-31, 
    mcerror=mcerror@entry=0x7ffef466c9e8) at ../../../../src/vfs/sftpfs/internal.c:63
#1  0x000055765764b2fd in sftpfs_chmod (vpath=<optimized out>, mode=16877, mcerror=mcerror@entry=0x7ffef466c9e8)
    at ../../../../src/vfs/sftpfs/internal.c:538
#2  0x000055765763e1fc in sftpfs_cb_chmod (vpath=<optimized out>, mode=<optimized out>) at ../../../../src/vfs/sftpfs/vfs_class.c:518
#3  0x00005576575c95a0 in mc_chmod (vpath=vpath@entry=0x557659555750, mode=16877) at ../../../lib/vfs/interface.c:252
#4  0x0000557657633479 in copy_dir_dir (tctx=tctx@entry=0x557659561b80, ctx=ctx@entry=0x55765958f500, 
    s=0x557659555820 "/home/alex/tmp/yii2-basic/basic/assets", d=<optimized out>, 
    d@entry=0x5576595808e0 "/sftp://pi@firefly/var/www/vhosts/yii2bm/assets", toplevel=toplevel@entry=1, move_over=move_over@entry=0, do_delete=0, 
    parent_dirs=0x55765954a240) at ../../../src/filemanager/file.c:2275
#5  0x000055765763544f in panel_operate (source_panel=0x557659555930, operation=operation@entry=OP_COPY, force_single=force_single@entry=0)
    at ../../../src/filemanager/file.c:2905
#6  0x000055765762ccbc in copy_cmd () at ../../../src/filemanager/cmd.c:802
#7  0x00005576575d34a5 in midnight_execute_cmd (sender=0x557659557410, command=22) at ../../../src/filemanager/midnight.c:1142
#8  0x00005576575b8170 in buttonbar_callback (w=0x557659557410, sender=<optimized out>, msg=<optimized out>, parm=<optimized out>, 
    data=<optimized out>) at ../../../lib/widget/buttonbar.c:171
#9  0x00005576575bc4fa in send_message (data=0x0, parm=1005, msg=MSG_HOTKEY, sender=0x0, w=<optimized out>)
    at ../../../lib/widget/widget-common.h:210
#10 dlg_try_hotkey (h=0x557659552490, h=0x557659552490, d_key=1005) at ../../../lib/widget/dialog.c:434
#11 dlg_key_event (d_key=1005, h=0x557659552490) at ../../../lib/widget/dialog.c:479
#12 dlg_process_event (h=0x557659552490, key=1005, event=<optimized out>) at ../../../lib/widget/dialog.c:1165
#13 0x00005576575bc850 in frontend_dlg_run (h=0x557659552490) at ../../../lib/widget/dialog.c:541
#14 dlg_run (h=0x557659552490) at ../../../lib/widget/dialog.c:1196
#15 0x00005576575d4706 in create_panels_and_run_mc () at ../../../src/filemanager/midnight.c:954
#16 do_nc () at ../../../src/filemanager/midnight.c:1770
#17 0x00005576575b0f1e in main (argc=<optimized out>, argv=<optimized out>) at ../../src/main.c:407

I just checked, the source directories and the directories created remote by mc have the same privilegies, the only difference is a different user:group (the user "pi" have full access in the destination directory)

​https://curl.haxx.se/mail/lib-2016-01/0104.html

sftpfs_chmod() is always failing only in the second do {...} while, after attrs.permissions = mode;

[zaytsev]

There are 4 call to libssh2_sftp_stat_ex in internal.c but you're right the only one failing is in sftpfs_stat, and with your patch -31:2 is gone.

Yes, this is absolutely disgusting copy & paste programming, how about properly rewriting all this stuff and covering it with tests? ;-P

I tried some copy and every backtrace looks almost the same, the failure seems to be from sftpfs_chmod() (on created directory maybe?)

Probably changing the permissions fails, and this error is not suppressed. Try (untested) patch:

diff --git a/src/vfs/sftpfs/internal.c b/src/vfs/sftpfs/internal.c
index 5377b66..2bdec11 100644
--- a/src/vfs/sftpfs/internal.c
+++ b/src/vfs/sftpfs/internal.c
@@ -470,7 +470,15 @@ sftpfs_chmod (const vfs_path_t * vpath, mode_t mode, GError ** mcerror)
                                     sftpfs_filename_buffer->len, LIBSSH2_SFTP_SETSTAT, &attrs);
         if (res >= 0)
             break;
-        else if (res != LIBSSH2_ERROR_EAGAIN)
+
+        if (res == LIBSSH2_ERROR_SFTP_PROTOCOL &&
+            libssh2_sftp_last_error(super_data->sftp_session) == LIBSSH2_FX_FAILURE)
+        {
+            res = 0;  /* need something like ftpfs_ignore_chattr_errors */
+            break;
+        }
+
+        if (res != LIBSSH2_ERROR_EAGAIN)
         {
             sftpfs_ssherror_to_gliberror (super_data, res, mcerror);
             return -1;

[sand]

Thumbs up! error -31:4 gone too :-D

I'm not 100% sure, I don't think is about permissions, the user connecting via sftp own the target directory with full perms +rwx.

Yes, this is absolutely disgusting copy & paste programming, how about properly rewriting all this stuff and covering it with tests? ;-P

I can take a look (or wasn't for me?) but not this year :-D
There's some automated test or are you talking about the old fashion?

Thanks a lot for the support! I'm gonna stress sftp a bit in the next days with some daily work.

Keep up the good work, and happy new year!

[zaytsev]

I'm not 100% sure, I don't think is about permissions, the user connecting via sftp own the target directory with full perms +rwx.

Could be UID / GID mismatch though...

I can take a look (or wasn't for me?) but not this year :-D

It was for anyone potentially willing to take a look, including you. The plan would be:

1. Merge Andrew's fix for segfault
2. Clean up the copy & paste WTF
3. Rebase patches by @and
4. Clean up my patches
   1. As you noted, I only patched functions that caused trouble in your particular case, others are still problematic, but maybe after eliminating copy & paste it will be straightforward.
   2. Think about a better solution for ignoring chattr errors; maybe introduce a generic option that will be valid for both ftpfs, sftpfs and whatever other VFSes? Needs input, e.g. from Andrew.
5. Add unit tests where possible

There's some automated test or are you talking about the old fashion?

We have some unit tests implemented with the check framework.

Thanks a lot for the support! I'm gonna stress sftp a bit in the next days with some daily work.

I'm glad we finally identified the reason behind these annoying -31 errors. Thanks for your help as well, hopefully someone will manage to clean up this mess. Happy New Year to you too!

[sand]

@zaytsev, challenge accepted :)

I took some time this evening, please see the attached internal.c, I've identified some pattern in the copy&paste code an tried to normalize it, beside the names of added functions/type/macro... let me know what you think about.

edit
I've cleaned up a bit more (not uploaded), please let me know when you have some time to check it, maybe a gist would be more pratical than upload here?

I'd like to instrument the test framework you were talking about but have no idea where to start.

Also I'd like to look into @pingu note about ciphers and speed but there also I've no idea where to start (how to change cipher configuration for mc?)

edit

Uploaded latest version of internal.c, looking at the code I wonder if all those sftpfs_fix_filename() need to be in the EAGAIN loop.

[zaytsev]

Thank you for your help, however, the attached file is of little use for us. Please commit your changes as separate patches and attach those to the ticket. We have a few wiki pages which explain how to work with git: GitGuideLines / WorkingGuideLines - patches from your local branch can be produced with git format-patch command. Regarding tests, all we have is in the tests directory. See the README file there. In as far as ciphers are concerned, sorry, I've got no clue; it would be awesome if sftp respected the ssh_config file, but I don't know if there are any facilities in the library you can make use of to that end.

[sand]

@zaytsev, before any official patch I'd like to know if the changes I did are aligned with your idea of cleanup, and eventually I'd really like suggestions/help naming the new functions/macro.

attached a patch, but is only for review (last commit of internal.c was 471ea781cacda6ddf324c614e2cf915d59325375).

while there are a lot of changes at once I think they are quite easy to read, if you prefer a set of progressive patches I can do that, will require some more work, just matter of time.

the API of internal.c isn't changed, I added only static functions and a couple of macro.

the sftpfs_* functions had almost all the same pattern:

• a prepare stage at the top which initialized some variable, I moved all this stuff to _sftpfs_op_init, struct vfs_s_super *super is used only there, so has been removed from all other function;

• at the bottom, within the do {...} while(res == EAGAIN), there was always the same check, if the error isn't EAGAIN map the ssh error to glib and return -1, optionally g_free a temporary path allocated, else call sftpfs_waitsocket and when it return check again for errors. I moved all this into _waitsocket_or_error, which eventually call the added macro mc_return_val_if_error_and_free, they both take into account the optional (non NULL) pointer to free.

sftpfs_lstat, sftpfs_stat and sftpfs_chmod now simply call _sftpfs_stat, only stat_type may be different.

only sftpfs_lstat and sftpfs_stat call _sftpfs_attr_to_buf which I think is self explanatory.

the patch for the for regression which caused segfault is within _sftpfs_attr_to_buf (below the comment)

[andrew_b]

I think we should use #3749 as base for this.

if (to_free != NULL)
   g_free(to_free);

@sand, this check is redundant. g_free() makes this check itself.

[sand]

@andrew_b thanks, done, I didn't know.

Also, as mentioned somewhere above, I think all fix statements could be moved outside of the do{...}while(EAGAIN), fixfname are declared as const, the libssh2_sftp_*_ex calls shouldn't touch its content, or I'm missing something?:

const char *fixfname;

        fixfname = sftpfs_fix_filename...

If you can confirm the patch is moving in the right direction I'll look into testing the changes.

edit

@andrew_b, only the changes below are related to #3749, then there are about two/three patches to ignore some proto error (specific to #3406), the rest is cleanup, perhaps some comment would be appropriate near the 3406's ignored proto errors (I'll add some) and test carefully that they generate errors when those are expected.

+#include "src/filemanager/ioblksize.h"          /* IO_BUFSIZE */

+    buf->st_blksize = IO_BUFSIZE;
+    buf->st_blocks = 1 + ((buf->st_size - 1) / buf->st_blksize);

edit

@andrew_b, @zaytsev, please take a look here: ​https://github.com/xenogenesi/mc/commits/fix-sftp-err-31, this is the minimal patchset needed to get rid of error -31 (segfault fix and and's EACCES patches included), re-integrated from scratch and tested (sorry for me was just easier/faster to fork and share from github)

If you want me to re-integrate the mess-cleanup over those in a set of progressive patches just let me know, I'd like to, but if you want to proceed in a different way, no problem, just let me know.

[andrew_b]

Branch: 3406_sftp_error_31
Initial changeset:822a0705a70fa44c7324923a5c9947faffb8a35e

Both and's patches are applied.
sand's big patch was split by several small patches and applied with some modification.

The work is not finished. Refactoring of internal.c only was made. Such refactoring is required for file.c and dir.c.

Currently we still have the -31 error at least in following two cases:

• remove file in sftp server;
• download file from sftp server.

[zaytsev]

Great work, awesome!

Nitpick:

The code in 38344f3fa68b0705d686747cd99348f6420e16b7:

 	545	        if (sftpfs_is_sftp_error (super_data->sftp_session, res, LIBSSH2_FX_NO_SUCH_FILE)) 
 	546	            return ENOENT; 
 	547	 
 	548	        if (sftpfs_is_sftp_error (super_data->sftp_session, res, LIBSSH2_FX_FAILURE)) 
 	549	        { 
 	550	            res = 0;            /* need something like ftpfs_ignore_chattr_errors */ 
 	551	            break; 
 	552	        } 

Should have been added in 10fedd5defa2d0c64bc8a1bd6d4f8c2e7a9fccf0 I think.

Question:

In 1740153a767e31523773046fffb7fe60c275f70f maybe

 	439	    if (res == -1 || res == EACCES || res == ENOENT) 
 	440	        return res; 

should be if (res < 0) ? Otherwise these lines will have always to be kept in sync with what sftpfs_internal_stat can return?

[andrew_b]

Replying to zaytsev:

Should have been added in 10fedd5defa2d0c64bc8a1bd6d4f8c2e7a9fccf0 I think.
[...]
should be if (res < 0) ?

Done.

[andrew_b]

Replying to zaytsev:

+        if (res == LIBSSH2_ERROR_SFTP_PROTOCOL &&
+            libssh2_sftp_last_error(super_data->sftp_session) == LIBSSH2_FX_NO_SUCH_FILE)
+            return ENOENT;
+

Well, I have read man 2 stat (really, I done it!) and found out that we can't use such way because:

• E* constants are positive, and if we want return negative values on error, we must return -EACCES -ENOENT, etc.
• on error, *stat should return (-1) and set errno. In our case errno is vfs_calss::verrno and E* conatsnts are values for that.

As a result of above, the branch should be reworked almost totally.

[zaytsev]

Oh, my fault, I've just copied what Andreas did, sorry about that :-/ So, these two commits:

• 7b668721531521b7477c4bb6cf648dbbab63ce81
• 22aa301fbaf152dfde57b24069eddb6b49b4aa22

are completely wrong, and, consequently, everything that follows them has to be updated :-|

Of course, you are right about E* being positive, but I think that we shouldn't rely on that, and the second approach you suggested must be followed.

Are you planning to work on this anytime in the future? I'm asking because currently the branch does not compile due to latest commits.

../../../../src/vfs/sftpfs/file.c: In function ‘sftpfs_file__handle_error’:

../../../../src/vfs/sftpfs/file.c:84:16: error: ‘EACCES’ undeclared (first use in this function)

../../../../src/vfs/sftpfs/file.c:84:16: note: each undeclared identifier is reported only once for each function it appears in

../../../../src/vfs/sftpfs/file.c:87:16: error: ‘ENOENT’ undeclared (first use in this function)

[andrew_b]

REMOVE REMOTE FILE: "-31: Failed opening remote file"

The root of the this problem is the passing the file to the opendir() callback:

panel_operate -> panel_operate_init_totals -> compute_dir_size -> do_compute_dir_size -> mc_opendir -> sftpfs_opendir -> libssh2_sftp_open_ex (..., fixfname, ..., LIBSSH2_SFTP_OPENDIR);

Here the "fixfname" is the file not a directory, but we try use it as directory.

Branch: 3406_sftp_error_31
Initial changeset:8d2bf20aa44912565dffe41deaa433467ad37668

Please review.

[zaytsev]

The refactoring looks awesome, it's exactly what I had in mind plus much more, however, I can't currently test this code. Could one of the long term error -31 sufferers please check this branch?

[aloy]

I've just tested this branch and it seems to fix the -31 error for me. Might need some more testing but it looks like the error is gone for me.

[andrew_b]

Thanks for feedback.
I'm going to merge this branch at the weekend.

[andrew_b]

Merged to master: [79b6a772fe3677f5f8752f317a689570db871359].

git log --pretty=oneline 218dcea..79b6a77

[lepi]

I get the same thing today (2020.11.13) - bulk operations are impossible : (

I'm running MC on Xubuntu 18.04.

$ mc --version
GNU Midnight Commander 4.8.19
Built with GLib 2.53.4
Using the S-Lang library with terminfo database
With builtin Editor
With subshell support as default
With support for background operations
With mouse support on xterm and Linux console
With support for X11 events
With internationalization support
With multiple codepages support
Virtual File Systems: cpiofs, tarfs, sfs, extfs, ext2undelfs, ftpfs, sftpfs, fish
Data types: char: 8; int: 32; long: 64; void *: 64; size_t: 64; off_t: 64;

The destination SFTP machine has also Xubuntu 18.04 and:
OpenSSH_7.6p1 Ubuntu-4ubuntu0.3, OpenSSL 1.0.2n 7 Dec 2017

[zaytsev]

Sorry, you have to upgrade your mc, see our Binaries page if you can't upgrade to a newer LTS.

[m_pashka]

I'm getting this error while moving or copying some *.img files over sftp. Almost all other files were moved without any problems. *.img files from another folder were also moved without errors. Then I've found two relatively small files, tried to rename them and still were getting the same error. So lookes like that is not file name related. See mc-err-files.zip ​https://www.dropbox.com/sh/9yp5f14yqef2mc8/AAB2RVd84XCfNpKX9nw6H9-Ja?dl=0. Probably it is related to sftp server - I use ubuntu 20.04 + vsftpd 3.0.3-12 on the server side.

$ lsb_release -a
No LSB modules are available.
Distributor ID: Ubuntu
Description: Ubuntu 22.04.2 LTS
Release: 22.04
Codename: jammy

$ uname -a
Linux usetech-rtc-pmukhataev 5.15.0-71-generic #78-Ubuntu SMP Tue Apr 18 09:00:29 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux

$ cat /proc/cpuinfo
cpu family : 23
model : 104
model name : AMD Ryzen 7 5700U with Radeon Graphics
stepping : 1
microcode : 0x8608103

$ mc --version
GNU Midnight Commander 4.8.27
Built with GLib 2.68.4
Built with S-Lang 2.3.2 with terminfo database
With builtin Editor and Aspell support
With subshell support as default
With support for background operations
With mouse support on xterm and Linux console
With support for X11 events
With internationalization support
With multiple codepages support
With ext2fs attributes support
Virtual File Systems:

cpiofs, tarfs, sfs, extfs, ext2undelfs, ftpfs, sftpfs, fish

Data types:

char: 8; int: 32; long: 64; void *: 64; size_t: 64; off_t: 64;

$ openssl version
OpenSSL 3.0.2 15 Mar 2022 (Library: OpenSSL 3.0.2 15 Mar 2022)

[andrew_b]

Replying to m_pashka:

I'm getting this error while moving or copying some *.img files over sftp. Almost all other files were moved without any problems. *.img files from another folder were also moved without errors. Then I've found two relatively small files, tried to rename them and still were getting the same error.

Do you have permissions to read/write those files in those folders?

Probably it is related to sftp server - I use ubuntu 20.04 + vsftpd 3.0.3-12 on the server side.

vsftpd is an FTP server not SFTP one.