file delete -force does not work on read-only directories

It might not be possible to delete the file in any case e.g. if it exists in a directory to which we have no write access and where we cannot change the permissions on the file.

Logged In: YES
user_id=32170

Also if 'cwd' is inside the directory, a delete will fail
(although we could even check for that and do a 'cd [file
dirname $dir]')

I assume from Jeff's comments that the idea of 'file
delete -force' is to try everything possible to get rid of
the file, including moving the cwd, changing permissions
etc?

Logged In: YES
user_id=72656

Yes, the idea would be to attempt other things to actually
delete the directory. This was mentioned to me by Scott
Stanton as the original intention (and thus the bug
report), but I'm not dead set on it.

Logged In: YES
user_id=79902

Following on from Vince's comment, things become even more
sticky when the 'cwd' of another process is inside the
directory. At that point, you're basically stuck. (I have
no idea if there are networked filesystems that support that
style of directory locking, but if there are you have the
situation where a blocking process might not even be on the
same computer. But even without that finesse, the point
still holds since you can't change the directory of other
processes underneath their feet...)

Anyway, hell-or-high-water isn't possible. Best effort
(probably up to about the level of 'rm -rf' I'd guess) is.

Logged In: YES
user_id=32170

The changing permisions piece is very easy to implement.
In 'DoRemoveDirectory' (tclUnixFCmd.c), we must simply add:

if (recursive) {
/* We should try to change permissions
* so this can be deleted */
chmod(path, XXX);
}

could someone enlighten me to what the correct value
of 'XXX' should be here?

Logged In: YES
user_id=32170

It would also be useful to have a new test which currently
fails on unix, but should pass after add 'chmod' as below --
- any takers?

Logged In: YES
user_id=79902

I'd guess the permissions to be 0700; don't care about
losing perms for other people as the dir is about to be
nuked anyway. Though if the delete then fails, we must
restore the perms to what they were before since the failure
might be due to another process owned by another user using
that dir as a working directory and making it fail in that
case is not too good. I think...

The classic way to test this is (I suppose) to create a
directory with a file in it, change its permissions to
something aggressively unreadable, and then attempt to
delete it. We can then force an unfixable failure by
putting a subprocess in there (which would also be good for
making sure that the permissions don't end up too screwed in
the case of a failure.)

Logged In: YES
user_id=32170

Attached patch to fix this and some other problems. Please
test on unix (and ideally add a test case...).

Vince.

Logged In: YES
user_id=79902

It strikes me as odd that we have an open fd for the
temporary file and then we immediately close it only to
reopen it again (as part of copying.) Wouldn't it be better
to change TclpTempFileName() to pass back both the name of
the file and a (binary read-write) channel with which to
access the file? One of the faults with using tmpnam() as I
understand it is that you can have an attacker create the
file with some nefarious content in-between getting the
filename (of a non-existing-at-the-time file) and creating
the file. If changed to produce both channel and filename,
then it is obvious that the name of the function needs to
change as well (to TclpCreateTempFile for example?)

Logged In: YES
user_id=32170

I considered doing that, perhaps combining the function
with the (existing) 'TclpCreateTempFile' which is above.

I appreciate the comment about 'tmpnam'; the approach in
the patch does have the benefit that the file actually
exists (even though we closed it), so I don't think an
attacker can mess with it. What happens next is that we
overwrite the existing file.

All in all I didn't see massive benefits from returning
both the name and the channel. While avoiding the 'open-
close-open' behaviour of the current code, in other ways it
complicates things (the Win/Mac ports must explicitly open
the temp file, the file handle must be converted to a
Tcl_Channel in all three platforms, we need to modify the
shared CrossFilesystemCopy, etc...)

Anyway, I don't feel that strongly about it, so am happy to
be argued back (!). Does the patch actually compile and
pass the tests?

Logged In: YES
user_id=79902

Patch doesn't even apply cleanly. (I think only one chunk
out of the whole lot applies at all!) :^(

Logged In: YES
user_id=32170

I assume you converted eols to unix, etc?

Logged In: YES
user_id=32170

This works in latest patch.