Ticket #3128 (new defect)

Opened 6 years ago

Last modified 7 months ago

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

Reported by: richlv Owned by:
Priority: blocker Milestone: Future Releases
Component: mc-core Version: 4.8.11
Keywords: Cc: howaboutsynergy@…
Blocked By: Blocking:
Branch state: no branch Votes for changeset:

Description

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

Change History

comment:1 Changed 6 years ago by richlv

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

comment:2 Changed 6 years ago by richlv

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

comment:3 Changed 6 years ago by andrew_b

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.

comment:4 follow-up: ↓ 5 Changed 5 years ago by 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

comment:5 in reply to: ↑ 4 Changed 5 years ago by andrew_b

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 for such complex way dd | tee | wc. Probably, this code should be fully rewritten.

comment:6 Changed 9 months ago by andrew_b

From ticket: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.

comment:7 Changed 9 months ago by andrew_b

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

comment:8 Changed 7 months ago by howaboutsynergy

  • Cc howaboutsynergy@… added
Note: See TracTickets for help on using tickets.