Opened 7 years ago

Last modified 7 years ago

#8321 new bug

Symbolic link is followed when replacing a file!

Reported by: Pete Owned by: nobody
Priority: normal Milestone: R1
Component: Applications Version: R1/Development
Keywords: Cc:
Blocked By: Blocking:
Has a Patch: no Platform: All

Description

I wanted to replace a link to a master copy of a file with an actual copy of an experimental version, so I dragged the new one on to the folder with the link. As usual, it asked me if I really wanted to replace the file, but when I clicked "Replace", it left the link in place and overwrote the master copy!! Ouch.

Fortunately I do have a backup of the master, but I don't think this is expected behaviour! (I am still on an older build -- hrev42415 -- but I don't see this having been reported before.)

Change History (9)

comment:1 Changed 7 years ago by Pete

Component: Add-Ons/TrackerKits/Storage Kit
Owner: changed from nobody to axeld

My apologies -- I'm posting too fast. This wasn't a drag and drop, but a Save from a FilePanel. I opened the filepanel in an editor (Eddie as it happens, but I just verified that the same is true with e.g. StyledEdit) and tried to save to the name of the link, but the file the link pointed to got overwritten instead.

comment:2 Changed 7 years ago by bonefish

Component: Kits/Storage KitApplications/StyledEdit
Owner: changed from axeld to korli
Version: R1/alpha3R1/Development

At the first glance I would say that is StyledEdit's fault. The way it opens the file is supposed to traverse symlinks (according to the BeBook). Theoretically O_NOTRAVERSE or O_NOFOLLOW could be specified (undocumented in the BeBook), but that would only make the operation fail, not replace the symlink. So the entry returned by BFilePanel would first have to be checked explicitly. Even better, the saving procedure should play it safe and first write to another file and, only if that succeeded, replace the old one.

comment:3 Changed 7 years ago by Pete

I'll. Be. Damned... I went into BeOS to prove you wrong (:-/) and find that it behaves in exactly the same way! And in all the applications I tried. It's hard to believe that I've been using BeOS since the beginning and never ran into it before!

I still think it's a dangerous convention, but it seems it is the convention, so I guess the ticket can be marked invalid. (Changing just StyledEdit would be worse.)

comment:4 Changed 7 years ago by bonefish

I find the ticket perfectly valid. If the user agrees to replace "foo", the application should replace "foo", not the file it refers to. The phrasing in the alert could be more accurate -- referring to "symbolic link 'foo'" in this case -- and ideally it could even offer an option to replace the file referred to by the symbolic link. And yes, this should be fixed in all applications. Pe works correctly, BTW (using save as new + remove old + rename, IIRC).

comment:5 in reply to:  4 ; Changed 7 years ago by Pete

Replying to bonefish:

I find the ticket perfectly valid. If the user agrees to replace "foo", the application should replace "foo", not the file it refers to. The phrasing in the alert could be more accurate -- referring to "symbolic link 'foo'" in this case -- and ideally it could even offer an option to replace the file referred to by the symbolic link. And yes, this should be fixed in all applications.

OK -- glad that you think it not quite right, too. I don't see it as the application's job to fix it, though. Shouldn't this be the FilePanel's responsibility? Having the alert give a choice of replacing either the link itself or the file it references might be the best route.

Pe works correctly, BTW (using save as new + remove old + rename, IIRC).

Hadn't tried that (don't have it on my BeOS) but when I looked it has a glitch or two too! Try this: create, say, a short text "testfile" in one folder, and create a link to it in another. Then drag that link onto Pe. Do a bit of editing and click on 'Save'...

It not only follows the link to change the original file, it sets the type of that file to be a symbolic link too!! The same happens with 'Save As...', unless you notice that the specified file type in the popup-menu is "Symbolic Link" and change it to something sensible; then things behave as I'd hope: the link gets replaced with the edited file.

comment:6 in reply to:  5 ; Changed 7 years ago by anevilyak

Replying to Pete:

OK -- glad that you think it not quite right, too. I don't see it as the application's job to fix it, though. Shouldn't this be the FilePanel's responsibility? Having the alert give a choice of replacing either the link itself or the file it references might be the best route.

No, it's the application. All the file panel does is select a file, it's in no way responsible for the subsequent modifications, and it does in fact give back a reference to the symbolic link. However, under normal circumstances, when one opens a file, the filesystem API calls deal with following the link and opening its target implicitly, as this allows apps to transparently work with symlinks without any extra work. In the case of an overwrite, what one normally does is open the file with a truncate flag, and then start writing the new contents. However, if the target is a symlink, this obviously doesn't quite result in the desired behavior, so the app does have to be smart enough to detect that situation when being asked to save/overwrite.

comment:7 in reply to:  6 Changed 7 years ago by Pete

Component: Applications/StyledEditKits/Storage Kit
Owner: changed from korli to axeld

Replying to anevilyak:

Replying to Pete:

OK -- glad that you think it not quite right, too. I don't see it as the application's job to fix it, though. Shouldn't this be the FilePanel's responsibility? Having the alert give a choice of replacing either the link itself or the file it references might be the best route.

No, it's the application. All the file panel does is select a file, it's in no way responsible for the subsequent modifications, and it does in fact give back a reference to the symbolic link.

I see your point. I'd forgotten that the FilePanel just passes the requested name for a save. (I'd also forgotten that a BFile does an automatic traverse if the name is a symlink -- even for a write.)

This makes things awkward. If some apps follow one convention and others another, the user is going to get very frustrated. And good luck getting all third-party apps converted!

At least, as Ingo suggests, the alert can be improved, at the expense of verbosity. Maybe:

   "The symlink "name" already exists in the specified folder.  
    Saving to it may overwrite the original file linked to.
    Do you really want to replace it?"

(I've changed the Component back to Storage Kit, as this problem is relevant to most apps, not just StyledEdit.)

comment:8 Changed 7 years ago by bonefish

Component: Kits/Storage KitApplications
Owner: changed from axeld to nobody

This really doesn't have anything to do with the storage kit. BFilePanel is in libtracker, if that's what you're thinking of.

I don't find your proposed text helpful at all, though. I believe the expected "Save as" logic is rather obvious: The application should replace the entry that the user selected, regardless of whether it is a symlink or a file. I would only change the file panel's alert text to use more correct phrasing in case of a symlink and maybe -- the phrasing in my earlier comment might have been misunderstandable -- add a third option to the alert to overwrite the file (i.e. there would be three instead of two options: "Cancel", "Overwrite symlink", "Overwrite file"). I'm not sure, if this is really necessary, though, since -- as written above -- I would find the obvious behavior to replace the symlink (why choose the symlink when you want to replace the referred to file?).

The "Save" behavior in case of opening a symlink is obvious, too: The actually opened file should be replaced. Actually the application should already explicitly resolve the symlink when opening (which would also be reflected in the document file path (e.g. in the window title)), so this isn't even a special case when saving.

In either case the application should use the safe method of saving: save in new file + remove old file + rename new file. In the "Save" case attributes need to be preserved (i.e. copied), which is where Pe apparently misbehaves a bit.

Changing the component to "Applications", since that's where the main changes need to happen.

comment:9 Changed 7 years ago by axeld

In any case, this is a rare enough situation that we can well live with the fact that 3rd party applications might not behave consistently in this regard.

The third button in the file panel would only work if the application handles the overwriting of the symlink correctly, which the file panel cannot know anything about. But that would just higher the pressure (as I said, it's rare, so the pressure will still be very low ;-)) on 3rd party apps to do it right, too.

Note: See TracTickets for help on using tickets.