Ticket #3128 (new defect)
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:2 Changed 11 years ago by richlv
...and the real reason - it does not see "permission denied" error and assumes file was uploaded successfully
comment:3 Changed 10 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 10 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 10 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 6 years 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.
looks like it might be actually connected to inability to store files over ssh at all. copying (even with attribute keeping disabled) fails, too