Bug 430585 - rename() falling over case insensitivity
Summary: rename() falling over case insensitivity
Alias: None
Product: kio-extras
Classification: Frameworks and Libraries
Component: Samba (show other bugs)
Version: unspecified
Platform: Neon Linux
: NOR normal
Target Milestone: ---
Assignee: Plasma Bugs List
Depends on:
Reported: 2020-12-19 16:42 UTC by Patrick Silva
Modified: 2021-03-04 17:49 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In: 21.04

screen recording (853.40 KB, video/webm)
2020-12-19 16:42 UTC, Patrick Silva

Note You need to log in before you can comment on or make changes to this bug.
Description Patrick Silva 2020-12-19 16:42:32 UTC
Created attachment 134209 [details]
screen recording

I can't reproduce this bug with local files.

1. have a folder in a Samba share containing these two files: file.txt and 
2. try to rename File1.txt to file1.txt

"File Already Exists" dialog unexpectedly shows up.
Watch the attached screen recording please.

File1.txt should be renamed to file1.txt without any dialog

Operating System: KDE neon Unstable Edition
KDE Plasma Version: 5.20.80
KDE Frameworks Version: 5.78.0
Qt Version: 5.15.2
Comment 1 Harald Sitter 2021-01-20 15:06:52 UTC
I wonder if that doesn't simply trip over case insensitivity. Supposedly samba should negotiate case sensitivity but I guess that won't matter much if we don't know what the outcome of that negotiation is.
Comment 2 Harald Sitter 2021-01-20 16:07:35 UTC
Case sensitivity is indeed the trouble there. Before the rename there's a stat call on the destination and if it exists the rename dialog is triggered. That is however falling into the case insensitivity trap as renaming a->A will have A existing because A=a.

This particular problem needs addressing in kio-extras and I threw together a hack that does some in depth comparison between src and dst to figure out if they might be the same file, alas, that revealed a problem with change notifications because windows servers send FilesRemoved before the FileRenamed which makes the file disappear from dolphin's model :@

That does kind of need fixing in dolphin first though (does that code even live in dolphin?)

signal time=1611157682.777775 sender=:1.3315 -> destination=(null destination) serial=19 path=/; interface=org.kde.KDirNotify; member=FilesRemoved
   array [
      string "smb://user@windev2004eval.local/Users/User/Desktop/New folder (6)/a"
signal time=1611157682.778322 sender=:1.3315 -> destination=(null destination) serial=20 path=/; interface=org.kde.KDirNotify; member=FileRenamed
   string "smb://user@windev2004eval.local/Users/User/Desktop/New folder (6)/a"
   string "smb://user@windev2004eval.local/Users/User/Desktop/New folder (6)/A"
signal time=1611157682.778640 sender=:1.3315 -> destination=(null destination) serial=22 path=/; interface=org.kde.KDirNotify; member=FilesChanged
   array [
      string "smb://user@windev2004eval.local/Users/User/Desktop/New folder (6)/A"

After that FilesChanged notification dolphin should have gone "what A?" and done a stat but it didn't so the file disappeared entirely.  In fact, perhaps already after FileRenamed since dolphin at that point doesn't know about neither a nor A.
If that can't be fixed in dolphin or where ever I can take a look at handling it lower in the kio-extras side but from what I recall of the notification code it's actually fairly horrible to put context on these events since one file operation translations into a multitude of events, twice as many for renaming/moving.
Comment 3 Harald Sitter 2021-03-04 11:16:16 UTC
Git commit 5abd1e8e981095b2c608042982f27a35f36d1b0c by Harald Sitter.
Committed on 04/03/2021 at 11:15.
Pushed by sitter into branch 'master'.

smb-notifier: do not send remove events on moving files

windows (but not samba interestingly) will send a REMOVE event before
the actual MOVE event this would mess with kio's dirlister's internal
state as it'd remove the file from the model and then never try to learn
about it again when it receives the move event. effectively this would
remove a file from the view in dolphin when renaming it as now both the
old name and the new name aren't in the dirlister model anymore.

to prevent this from happening a similar hack is applied to the event
order as with move merging.

when a remove arrives it's not immediately emitted but queued for up to
1000ms if a move event also arrives during that time frame the remove is
entirely discarded and we are left with only the move.
if any other event arrives the remove is immediately sent.
if nothing else arrives the timer runs out and the remove is sent.

so worst case a remove is only represented in the GUI after 1s, best
case it's more or less same as before with the added benefit that
renaming doesn't make files disappear

M  +91   -3    smb/kded/notifier.cpp

Comment 4 Harald Sitter 2021-03-04 11:16:23 UTC
Git commit ed0a8e905df6c5e0ede3486f38afdb555ef80ce0 by Harald Sitter.
Committed on 04/03/2021 at 11:15.
Pushed by sitter into branch 'master'.

smb: do not assume rename files are different based on name

samba is transparently supporting case sensitivity/insensitivity based
on server capabilities so 'A' and 'a' may be the same file or not. To
that end when a rename operation would change the capitalization of a
file we need to do some extra work to figure out if that renaming would
constitute an overwrite or not. Specifically we'll need to stat the
source file and then compare the inode and device returned by libsmb to
figure out if they are the same file. If they are then this is an
in-place rename, not an overwrite and we'll skip over the error
conditions to do with the dst file already existing

M  +34   -3    smb/kio_smb_dir.cpp