dataloss: moving a file and skipping upon problem deletes original file

[mc-butler]

Important

This issue was migrated from Trac:

Origin	https://midnight-commander.org/ticket/3128
Reporter	richlv (richlv@….net)
Mentions	howaboutsynergy@….me

move a file to some location that causes a problem (for example, unable to chown it on the other side over ssh/shell link).

when mc complains, choose 'skip'.

target file is not closed, but original file is still deleted.

reproduced with 4.8.4 in opensuse 13.1 and 4.8.11 in debian testing.

opensuse report : https://bugzilla.novell.com/show_bug.cgi?id=856501

[mc-butler]

Changed by richlv (richlv@….net) on Dec 20, 2013 at 13:31 UTC (comment 1)

looks like it might be actually connected to inability to store files over ssh at all. copying (even with attribute keeping disabled) fails, too

[mc-butler]

Changed by richlv (richlv@….net) on Dec 20, 2013 at 13:36 UTC (comment 2)

...and the real reason - it does not see "permission denied" error and assumes file was uploaded successfully

[mc-butler]

Changed by andrew_b (@aborodin) on Mar 16, 2014 at 11:40 UTC (comment 3)

I'm absolutely unable to reproduce this bug. Everything works fine for me.
I have the "Permission denied" message if file can't be created in the remote unwritable directory. I don't have any deleted files in case of permissions denied.

Please provide detailed step-by-step testcase.

[mc-butler]

Changed by richlv (richlv@….net) on Dec 25, 2014 at 14:33 UTC (comment 4)

sorry for missing that question. this has been reproduced by others and suse has disabled fish support completely because of this. see http://lists.opensuse.org/opensuse/2014-12/msg01128.html for more detail

[mc-butler]

Changed by andrew_b (@aborodin) on Dec 29, 2014 at 11:47 UTC (comment 4.5)

Replying to richlv:

sorry for missing that question. this has been reproduced by others and suse has disabled fish support completely because of this. see http://lists.opensuse.org/opensuse/2014-12/msg01128.html for more detail

$ mkdir /tmp/foo
$ chmod a-w /tmp/foo
$ cp whatever foo_to_be_moved_but_will_be_deleted
$ mc . .

- start your sshd allowing password based login locally (that's
another problem with ssh/sftp in mc which can be ignored for this)

- In mc now choose shell-link/fish to connect to localhost, change to
localhost:/tmp/foo as target (and stay local with the "source").

- Now, as source, select the file foo_to_be_moved_but_will_be_deleted.

- Then "Move" the file from "local" to "shell-link/fish" via F6.
You'll get an error (no write permission), as expected. BUT YOUR
SOURCE foo_to_be_moved_but_will_be_deleted WILL BE GONE TOO as if
the "move" had succeeded! NOT good, eh?

The destination directory is write-protected. This is key moment here.

We have send and append helper scripts with the following code:

     12     while [ $FISH_FILESIZE -gt 0 ]; do
     13         cnt=`expr \\( $FISH_FILESIZE + $bsl \\) / $bss`
     14         n=`dd bs=$bss count=$cnt | tee -a "${FILENAME}" | wc -c`
     15         FISH_FILESIZE=`expr $FISH_FILESIZE - $n`
     16     done

There is no error handling here.

When tee -a is unable to write to destination file, it returns "not 0". But tee is not the final stage of the pipe, and we can't get it's return value (like $?). Bash provides $PIPESTATUS, other shells don't. We must provide the portable shell code here.

There was some reason [407497f] for such complex way dd | tee | wc. Probably, this code should be fully rewritten.

[mc-butler]

Changed by andrew_b (@aborodin) on Feb 15, 2019 at 16:57 UTC (comment 6)

From #3963 comment 2:

I can confirm.

fish stor/send helper function don't catch error at touch aka '>' and tee -a failure.
Following modified send/stor helper function report 500 in case of a failure.

#STOR $FISH_FILESIZE $FISH_FILENAME
FILENAME="/${FISH_FILENAME}"
echo "### 001"
if > "${FILENAME}"; then
    bss=4096
    bsl=4095
    if [ $FISH_FILESIZE -lt $bss ]; then
        bss=1;
        bsl=0;
    fi
    s=200
    while [ $FISH_FILESIZE -gt 0 ]; do
        cnt=`expr \\( $FISH_FILESIZE + $bsl \\) / $bss`
        o=`dd bs=$bss count=$cnt | tee -a "${FILENAME}"`
        if [ $? -ne 0 ] ; then
            s=500
            break
        fi
        n=`echo $o | wc -c`
        FISH_FILESIZE=`expr $FISH_FILESIZE - $n`
    done
    echo "### $s"
else
    echo "### 500"
fi

Sadly this is not enough, mc code don't handle error return of store function call I guess.

[mc-butler]

Changed by andrew_b (@aborodin) on Feb 15, 2019 at 17:00 UTC (comment 7)

Ticket #3963 has been marked as a duplicate of this ticket.

[mc-butler]

Changed by howaboutsynergy on May 6, 2019 at 21:14 UTC (comment 8)

• Cc set to howaboutsynergy@….me