Tcl Source Code

View Ticket
Login
Ticket UUID: 1193497
Title: file writable lies for certain XP directories
Type: Bug Version: obsolete: 8.5a4
Submitter: fvogel Created on: 2005-05-01 21:26:25
Subsystem: 37. File System Assigned To: vincentdarley
Priority: 9 Immediate Severity:
Status: Closed Last Modified: 2014-02-19 20:31:46
Resolution: Fixed Closed By: vincentdarley
    Closed on: 2006-03-21 09:45:22
Description:
Windows XP SP2 french, Tcl/Tk cvs HEAD

file writable "C:/Documents and 
Settings/<account>/Mes Documents/mydir"
returns 1 if mydir does exist, or 0 if not.
This is correct.

file writable "C:/Documents and Settings/"
returns also 1, which is correct.

But:
file writable "C:/Documents and 
Settings/<account>/Mes Documents"
returns 0 but it should be 1

<account> is my account name (with administrator 
rights), this directory of course exists and I'm allowed to 
write files in it.
User Comments: vincentdarley added on 2006-03-21 16:45:21:
Logged In: YES 
user_id=32170

applied.

fvogelnew1 added on 2006-03-21 04:06:04:

File Deleted - 171601: 



File Added - 171605: patch2_tclWinTest.c

fvogelnew1 added on 2006-03-21 04:06:03:
Logged In: YES 
user_id=1245417

Here you are!
Hope it's correct now.

Thanks,
Francois

vincentdarley added on 2006-03-21 03:31:58:
Logged In: YES 
user_id=32170

That patch leaves an unreferenced local variable and gives
me a warning, plus it seems to adjust a bunch of formatting.
 Can you please try compiling with OPTS=symbols and provide
a minimal unified diff that solves the issue without any
warnings?

thanks!

Vince.

fvogelnew1 added on 2006-03-21 02:46:47:

File Added - 171601: patch_tclWinTest.c

fvogelnew1 added on 2006-03-21 02:46:45:
Logged In: YES 
user_id=1245417

Vince,

Jason is right. Please find attached a patch for 
tclWinTest.c that make things work for me.

Francois

jfg118 added on 2006-03-20 09:24:24:
Logged In: YES 
user_id=1266631

The everyone lookup is not quite right... You need to delete
the userDomain and lookupAccountName.  

This should be the entire lookup :

 /* Get the "Everyone" SID */
userSid = ckalloc(getSidLengthRequiredProc(1));
initializeSidProc( userSid, &userSidAuthority, 1);
*(getSidSubAuthorityProc( userSid, 0)) = SECURITY_WORLD_RID;

/* If curAclPresent == false then curAcl and curAclDefaulted
not valid */

You will be able to delete the userDomain buffer and delete
the lookupAccountName DLL lookup.

vincentdarley added on 2006-03-20 05:50:23:
Logged In: YES 
user_id=32170

Fixed that and applied to 8.4 branch.

fvogelnew1 added on 2006-03-20 02:39:27:
Logged In: YES 
user_id=1245417

Btw, Vince, I would be *really* interested to have this 
bug fixed in the upcoming 8.4.13. Could you perhaps 
backport this to the 8.4 branch?

Many thanks,
Francois

fvogelnew1 added on 2006-03-20 02:26:13:
Logged In: YES 
user_id=1245417

Vince,

Thanks for your update of the tests.

However, for me they still fail because you didn't update 
tclWinTest.c according to Jason's proposal below dated 
2006-03-18 01:08. Did you overlook this or do you have a 
good reason for not doing it?

Thanks,
Francois

vincentdarley added on 2006-03-19 01:17:41:
Logged In: YES 
user_id=32170

Committed the recommended changes to cvs HEAD.

fvogelnew1 added on 2006-03-18 19:35:32:
Logged In: YES 
user_id=1245417

I've just ran the complete test suite. Here are the 
results:

all.tcl:        Total   22459   Passed  21507   Skipped 
948     Failed  4
Sourced 134 Test Files.
Files with failing tests: expr-old.test expr.test 
parseExpr.test

Looking at the failed test cases, they have nothing to do 
with this bug or patch or testchmod.

Francois

fvogelnew1 added on 2006-03-18 19:34:34:
Logged In: YES 
user_id=1245417

I've just ran the complete test suite. Here are the 
results:

all.tcl:        Total   22459   Passed  21507   Skipped 
948     Failed  4
Sourced 134 Test Files.
Files with failing tests: expr-old.test expr.test 
parseExpr.test

Looking at the failed test cases, they have nothing to do 
with this bug or patch or testchmod.

Francois

fvogelnew1 added on 2006-03-18 17:25:41:
Logged In: YES 
user_id=1245417

Failing tcltest-8.4:

It fails because tcltest.test line 560 says (for windows):
   catch {file attributes $notWriteableDir -readonly 1}

Since now the writeability is no more checked by the 
readonly attribute, this is wrong. It should perform a 
testchmod instead:
   testchmod 000 $notWriteableDir

Now, *no* test related to the patch does fail anymore for 
me.

As far as I can say, this bug can be closed after having 
committed the changes listed in this message and in the 
previous three below.

Francois

fvogelnew1 added on 2006-03-18 16:38:31:
Logged In: YES 
user_id=1245417

I would rewrite tests 7.11 and 7.19 for winFCmd.test as 
follows:

test winFCmd-7.11 {TraverseWinTree: call TraversalCopy: 
DOTREE_PRED} {win} {
    cleanup
    file mkdir td1
    createfile td1/tf1 tf1
    testchmod 000 td1
    testfile cpdir td1 td2
    list [file exists td2] [file writable td2]
} {1 1}

Francois

fvogelnew1 added on 2006-03-18 15:58:33:
Logged In: YES 
user_id=1245417

Thanks Jason, this works for me. Clever idea, indeed.
Vince you have my imprimatur (in case you need it?) to 
commit this change.

Thanks,
Francois

jfg118 added on 2006-03-18 07:08:00:
Logged In: YES 
user_id=1266631

I found this alternative way to lookup the Everyone sid
without the string representation.

http://support.microsoft.com/default.aspx?scid=kb;en-us;111543

>> Add these definitions
SID_IDENTIFIER_AUTHORITY userSidAuthority =
SECURITY_WORLD_SID_AUTHORITY;

typedef DWORD (WINAPI *getSidLengthRequiredDef) ( UCHAR );
typedef BOOL (WINAPI *initializeSidDef) ( PSID,
PSID_IDENTIFIER_AUTHORITY, BYTE );
typedef PDWORD (WINAPI *getSidSubAuthorityDef) ( PSID, DWORD );

static getSidLengthRequiredDef getSidLengthRequiredProc;
static initializeSidDef initializeSidProc;
static getSidSubAuthorityDef getSidSubAuthorityProc;

>>> Add to DLL init section
getSidLengthRequiredProc = (getSidLengthRequiredDef)
  GetProcAddress(hInstance, "GetSidLengthRequired");
initializeSidProc = (initializeSidDef)
  GetProcAddress(hInstance, "InitializeSid");
getSidSubAuthorityProc = (getSidSubAuthorityDef)
  GetProcAddress(hInstance, "GetSidSubAuthority");
>>> Be sure to update the if check to make sure all are loaded.

>>Replace the Everyone lookup
/* Get the "Everyone" SID */
userSid = ckalloc(getSidLengthRequiredProc(1));
initializeSidProc( userSid, &userSidAuthority, 1);
*(getSidSubAuthorityProc( userSid, 0)) = SECURITY_WORLD_RID;

fvogelnew1 added on 2006-03-18 06:40:23:
Logged In: YES 
user_id=1245417

Hi,

1.
I've seen you fixed both typos in tclWinTest.c, thanks.


2.
I found the reason why fCmd.test spits errors for me and 
not for Vince.

Running tcltest in debug mode it turns out that the 
failing tests from fCmd.c are failing because in line 654 
of tclWinTest.c GetLastError() has value 1332 for me, 
which is ERROR_NONE_MAPPED (No mapping between account 
names and security IDs was done.)

This happens on my system because at this point 
TestplatformChmod tries to find permissions for the 
EVERYONE SID. It fails because I'm the only user allowed 
to have access to the directory that was just created. In 
turn, this is because the directory inherited the 
permissions from its parent. There is no "EVERYONE" 
account for that directory. I can create one, but since I 
have a french XP, its name is "Tout le monde".

So to make tclWinTest.c work for me, I must change line 
505 from
    static const char everyoneBuf[] = "EVERYONE";
to
    static const char everyoneBuf[] = "Tout le monde";

Then *no* test from fCmd.test is failing anymore.

So it seems that the locale comes into play while checking 
for permissions in TestplatformChmod.

Is there any way to make this work automatically in your 
opinion?
I mean, there should be a way to retrieve the name of 
the "EVERYONE" account from the locale, and set the 
everyoneBuf character string accordingly.

Anyway, the patch is correct and the tests from fCmd.test 
are also correct.


3.
Failing tcltest-8.4: not yet looked at it. Probably 
tomorrow.


4.
Failing tests from winFCmd.test: only two tests are now 
failing (7.11 and 7.19). Here the error is not due to 
testchmod in this case (it succeeds). Rather, from winFCmd-
7.11 or 7.19 contents (btw, I see no difference between 
those two tests?):
% file mkdir td1
% testchmod 000 td1
% createfile td1/tf1 tf1
couldn't open "td1/tf1": permission denied

This is actually because createfile tries to open file tf1 
in td1 for writing.
Here is what proc createfile does first:
% set file td1/tf1
td1/tf1
% set f [open $file w]
couldn't open "td1/tf1": permission denied

This fails because of reason 2. exhibited by Jason below:
>Testcase does a chmod 000 and expects the directory to 
>be writable

I believe the testcase should be corrected as Jason 
suggested:
>I think the test should really be creating
>another sub directory under the first one before chmod 000
>and then try to write to it.  The testcases appear to be
>checking that a non readable directory can be traversed
>through.


5.
Is there any further failing test that I'm not aware of (I 
mean a test that seems to fail because of Jason's patch)?


Francois

fvogelnew1 added on 2006-03-17 06:59:50:
Logged In: YES 
user_id=1245417

Another typo in tclWinTest.c

Line 607 should read:
Tcl_MutexUnlock(&initializeMutex);

Anyway, this does not make any difference in:
1. the ability to compile (??)
2. the (failing) tests results

Francois

fvogelnew1 added on 2006-03-17 06:42:57:
Logged In: YES 
user_id=1245417

I'm running tcltest (double-click in its name in the 
Windows explorer in win/release, then cd ../../tests and 
finally source fCmd.test for instance.

nobody added on 2006-03-17 06:36:13:
Logged In: NO 

Hmm, if testchmod can't find the directory for you, what
path are you running tcl from?  I did notice some weirdness
in testchmod wrt it using the 'translated' rather than
'native' path, but I didn't want to make too large changes...

fvogelnew1 added on 2006-03-17 06:31:28:
Logged In: YES 
user_id=1245417

Strange you don't see any problem. I also use cvs HEAD on 
XP... I tried to cvs update again but now it asks me for a 
password for the first time! Well, enough for tonight.

Btw, I noticed a typo in tclWinTest.c. Line 570 reads
TCL_DECLARE_MUTEX(initialzeMutex)
while it should read
TCL_DECLARE_MUTEX(initializeMutex)

Francois

nobody added on 2006-03-17 06:20:17:
Logged In: NO 

Can you perhaps provide a patch to fix these issues, bearing
in mind that we need to not perturb the test suite on unix?

Note: I don't see any failures in fCmd.test on my Windows XP
machine with cvs HEAD.

Vince.

fvogelnew1 added on 2006-03-17 06:06:27:
Logged In: YES 
user_id=1245417

So here is what I have found now:

% source fCmd.test

==== fCmd-9.4 file rename: comprehensive: dir to new name 
FAILED
==== Contents of test case:

    cleanup
    file mkdir td1 td2
    testchmod 555 td2
    file rename td1 td3
    file rename td2 td4
    list [lsort [glob td*]] [file writable td3] [file 
writable td4]

---- Test generated error; Return code was: 1
---- Return code should have been one of: 0 2
---- errorInfo: td2: no such file or directory
    while executing
"testchmod 555 td2"
    ("uplevel" body line 4)
    invoked from within
"uplevel 1 $script"
---- errorCode: POSIX ENOENT {no such file or directory}
==== fCmd-9.4 FAILED

REASON: Problems seems to be in testchmod:

% file mkdir td1 td2
% testchmod 555 td2
td2: no such file or directory
% file exists td2
1

Problem is apparently that testchmod cannot find td2 that 
was just created??


==== fCmd-9.6 file rename: comprehensive: dir to self 
FAILED

==== fCmd-9.9 file rename: comprehensive: dir to non-empty 
dir FAILED

==== fCmd-9.11 file rename: comprehensive: dir to new name 
and dir FAILED

==== fCmd-10.3.1 file copy: comprehensive: dir to new name 
FAILED

==== fCmd-10.5 file copy: comprehensive: dir to empty dir 
FAILED

==== fCmd-10.6 file copy: comprehensive: dir to non-empty 
dir FAILED

==== fCmd-10.8.1 file rename: comprehensive: dir to new 
name and dir FAILED


All the 8 failing tests in fCmd.test are failing because 
of testchmod that can't find the directory that was just 
created before.


---------------------

Now:
% source tcltest.test

==== tcltest-5.5 InitConstraints: list of built-in 
constraints FAILED
==== Contents of test case:
 lsort [array names ::tcltest::testConstraints]
---- Result was:
2000orNewer 95 95or98 98 asyncPipeClose dontCopyLinks 
eformat emptyTest exec fil
eSharing foundGroup hasIsoLocale interactive knownBug 
linkDirectory linkFile mac
 macCrash macOnly macOrPc macOrUnix macOrWin nonBlockFiles 
nonPortable notFileSh
aring notNetworkFilesystem notRoot nt pc pcCrash pcOnly 
root singleTestInterp so
cket stdio tempNotMac tempNotPc tempNotUnix tempNotWin 
testchmod testsetplatform
 testvolumetype unix unixCrash unixExecs unixOnly unixOrPc 
unixOrWin userInterac
tion win winCrash winOnly xdev
---- Result should have been (exact matching):
95 98 asyncPipeClose eformat emptyTest exec hasIsoLocale 
interactive knownBug ma
c macCrash macOnly macOrPc macOrUnix macOrWin 
nonBlockFiles nonPortable notRoot
nt pc pcCrash pcOnly root singleTestInterp socket stdio 
tempNotMac tempNotPc tem
pNotUnix tempNotWin unix unixCrash unixExecs unixOnly 
unixOrPc unixOrWin userInt
eraction win winCrash winOnly
==== tcltest-5.5 FAILED

REASON: Vince perhaps forgot to update the constraints 
list?


==== tcltest-8.4 tcltest a.tcl -tmpdir notWriteableDir 
FAILED
==== Contents of test case:

    slave msg $a -tmpdir $notWriteableDir
    string match {*not writeable*} $msg

---- Result was:
0
---- Result should have been (exact matching):
1
==== tcltest-8.4 FAILED

REASON: hmmm... don't know...


About tests from winFcmd.test, again there is for all 7 
failing tests a testchmod 000 xxx that does not find the 
directory it is supposed to find.

Therefore I'm in agreement with Jason:

>1. Windows does not preserve ACL on copy it instead 
>inherits
>the permissions from the parent.
>2. Testcase does a chmod 000 and expects the directory to 
>be
>writable.  I think the test should really be creating
>another sub directory under the first one before chmod 000
>and then try to write to it.  The testcases appear to be
>checking that a non readable directory can be traversed
>through.

Shouldn't the tests be updated for Windows?


>3. Using file attribute -readonly to make a directory
>readonly.  The test does not appear to be valid for 
windows . 

To which test are you referring to here?

Francois

vincentdarley added on 2006-03-16 06:10:40:
Logged In: YES 
user_id=32170

Added proper constraints so those two (new) tests don't run on Unix.

dgp added on 2006-03-15 03:29:07:
Logged In: YES 
user_id=80530


New test failures on the
HEAD on Solaris.  Looks like
these patches are to blame:

==== fCmd-10.3.1 file copy: comprehensive: dir to new name
FAILED
==== Contents of test case:

    # On Windows with ACLs, copying a directory is defined
like this
    cleanup
    file mkdir [file join td1 tdx]
    file mkdir [file join td2 tdy]
    testchmod 555 td2
    file copy td1 td3
    file copy td2 td4
    set msg [list [lsort [glob td*]] [glob -directory td3
t*]  [glob -directory td4 t*] [file writable td3] [file
writable td4]]
    testchmod 755 td2
    testchmod 755 td4
    set msg

---- Result was:
{td1 td2 td3 td4} td3/tdx td4/tdy 1 0
---- Result should have been (exact matching):
{td1 td2 td3 td4} td3/tdx td4/tdy 1 1
==== fCmd-10.3.1 FAILED


==== fCmd-10.8.1 file rename: comprehensive: dir to new name
and dir FAILED
==== Contents of test case:

    # On Windows with ACLs, copying a directory is defined
like this
    cleanup
    file mkdir td1
    file mkdir td2
    file mkdir td3
    testchmod 555 td2
    file copy td1 [file join td3 td3]
    file copy td2 [file join td3 td4]
    list [lsort [glob td*]] [lsort [glob -directory td3 t*]]
 [file writable [file join td3 td3]] [file writable [file
join td3 td4]]

---- Result was:
{td1 td2 td3} {td3/td3 td3/td4} 1 0
---- Result should have been (exact matching):
{td1 td2 td3} {td3/td3 td3/td4} 1 1
==== fCmd-10.8.1 FAILED

Doesn't make sense that fixing a bug
on Windows will break things on unix.
Please take care.

fvogelnew1 added on 2006-03-15 02:55:14:
Logged In: YES 
user_id=1245417

Vince,

Thanks. However I don't see the changes while browsing the 
cvs through ViewCVS at SourceForge. Most recent commit in 
HEAD still has "remove previous patch for 'file writable'" 
as comment (4 days ago). I tried to cvs update but got 
nothing new. Is there some replication process that could 
delay visibility?

Francois

nobody added on 2006-03-15 02:34:54:
Logged In: NO 

I've updated cvs head to my latest.

Vince.

jfg118 added on 2006-03-15 00:47:37:
Logged In: YES 
user_id=1266631

Hi Francois, 

Vince reproduced the same results as I obtained.  You can
see the results in the comments.

You should be able to apply the two changed files the the
Tcl 8.4.12 source, or do a simple merge to CVS HEAD. The
code changes are all isoloted and should "plug in" easily.
Besure to redownload both patches before applying. 

The results came down to three different types over 5 failures :

1. Windows does not preserve ACL on copy it instead inherits
the permissions from the parent.
2. Testcase does a chmod 000 and expects the directory to be
writable.  I think the test should really be creating
another sub directory under the first one before chmod 000
and then try to write to it.  The testcases appear to be
checking that a non readable directory can be traversed through.
3. Using file attribute -readonly to make a directory
readonly.  The test does not appear to be valid for windows . 

--Jason

fvogelnew1 added on 2006-03-14 23:58:15:
Logged In: YES 
user_id=1245417

Vince,

I would be happy to test, but I'm afraid I'm a bit lost in 
the changes. Could you perhaps apply all the most recent 
patches to HEAD (tclWinFile and testchmod patch, and 
whatever is up-to-date), so that I can try on a consistent 
version?

Thanks,
Francois

vincentdarley added on 2006-03-14 21:43:48:
Logged In: YES 
user_id=32170

And tcltest-8.4

tcltest.test

==== tcltest-8.4 tcltest a.tcl -tmpdir notWriteableDir FAILED
==== Contents of test case:

    slave msg $a -tmpdir $notWriteableDir
    string match {*not writeable*} $msg

---- Result was:
0
---- Result should have been (exact matching):
1
==== tcltest-8.4 FAILED

thread.test
timer.test
tm.test
trace.test
unixFCmd.test
unixFile.test
unixInit.test
unixNotfy.test
unknown.test
unload.test
uplevel.test
upvar.test
utf.test
util.test
var.test
while-old.test
while.test
winConsole.test
winFCmd.test

==== winFCmd-7.11 TraverseWinTree: call TraversalCopy:
DOTREE_PRED FAILED
==== Contents of test case:

    cleanup
    file mkdir td1
    testchmod 000 td1
    createfile td1/tf1 tf1
    testfile cpdir td1 td2
    list [file exists td2] [file writable td2]

---- Test generated error; Return code was: 1
---- Return code should have been one of: 0 2
---- errorInfo: couldn't open "td1/tf1": permission denied
    while executing
"open $file w"
    (procedure "createfile" line 2)
    invoked from within
"createfile td1/tf1 tf1"
    ("uplevel" body line 5)
    invoked from within
"uplevel 1 $script"
---- errorCode: POSIX EACCES {permission denied}
==== winFCmd-7.11 FAILED


==== winFCmd-7.19 TraverseWinTree: call TraversalCopy:
DOTREE_POSTD FAILED
==== Contents of test case:

    cleanup
    file mkdir td1
    testchmod 000 td1
    createfile td1/tf1 tf1
    testfile cpdir td1 td2
    list [file exists td2] [file writable td2]

---- Test generated error; Return code was: 1
---- Return code should have been one of: 0 2
---- errorInfo: couldn't open "td1/tf1": permission denied
    while executing
"open $file w"
    (procedure "createfile" line 2)
    invoked from within
"createfile td1/tf1 tf1"
    ("uplevel" body line 5)
    invoked from within
"uplevel 1 $script"
---- errorCode: POSIX EACCES {permission denied}
==== winFCmd-7.19 FAILED

vincentdarley added on 2006-03-14 21:42:05:
Logged In: YES 
user_id=32170

With the latest patches, I still see a couple of new test
failures: winFCmd-7.11 and winFCmd-7.19.

What's your take on those Francois?

dkf added on 2006-03-14 15:57:08:
Logged In: YES 
user_id=79902

Defining an -acl attribute for files is a *good* idea. It'll
need a TIP later on down the road, but that won't be hard to
do. The code to implement it should be written first though;
leave the worrying about the process to the TCT. :-)

jfg118 added on 2006-03-14 08:36:38:
Logged In: YES 
user_id=1266631

From the Tcl documentation : -readonly gives the value or
sets or clears the readonly attribute of the file.

I would think the test should be querying the -readonly
attribute not necessarily "assuming" it makes a directory
non-writable.

The problem with making -readonly change the ACL list is it
is highly dependent on the user.  For user A and B a
different ACL changes may need to be applied.  To make the
patch simplistic for testing, the testchmod command simply
creates a "Deny Entry" for "Everyone" and then removes the
exact "Deny Entry" to make it writable as it was. However
adding various other groups or users could accomplish the
same thing.

I much rather see an extension to control the acl file
operations more directly. E.g. something like -acl
<user|group>[+-=]<read|write|etc...>[|<attr>]...,
encompassing the multitude of windows ACL attributes... The
code in the patch could be adapted to do more generic changes.

nobody added on 2006-03-14 06:56:07:
Logged In: NO 

Should 'file attributes -readonly' use the ACL stuff too, so that it does what it 
claims to do?

Vince.

jfg118 added on 2006-03-14 06:51:19:
Logged In: YES 
user_id=1266631

I have attached a patch for generic/testTcl.c on Duplicate
bug 1344540. The patch alters the testchmod for directories
to apply an ACL change to make the directory non-writable.

With the change only the five tests below fail with the
patch applied to fix this bug.  At this point I would say
the the patch is functioning properly as designed.  Let me
know what else needs to be done to get this patch applied to
CVS HEAD and the 8.4 branch.

Testcase Analysis:
fCmd-10.3 file copy: comprehensive: dir to new name FAILED
fCmd-10.8 file rename: comprehensive: dir to new name and
dir FAILED

>>> FAIL REASON: the directory permissions are not preserved
on copy.  td4 is writable.  This is how ACL's work on
windows. They are for the most part inherited from the
parent.  Are these tests valid for WinNT? If so perhaps a
second "testchmod" could be applied after the copy to the
new directory.

--

winFCmd-7.11 TraverseWinTree: call TraversalCopy:
DOTREE_PRED FAILED
winFCmd-7.19 TraverseWinTree: call TraversalCopy:
DOTREE_POSTD FAILED

>>> FAIL REASON: testchmod 000 makes the td1 directory not
writable, and you cannot create td1/tf1.  The testcases
appears to be invalid.

--
tcltest-8.4 tcltest a.tcl -tmpdir notWriteableDir FAILED
>>> FAIL REASON: the {file attributes $notWriteableDir
-readonly 1} command does not make the directory non
writable. Perhaps this this test invalid for WinNT as the
readonly attribute does not make the directory non writeable.

===========================================================
Detailed Test Failure information (LOGS):


==== fCmd-10.3 file copy: comprehensive: dir to new name FAILED
>>> FAILS BECAUSE the directory permissions are not
preserved on copy.  td4 is writable.  This is how ACL's work
on windows. They are for the most part inherited from the
parent.
==== Contents of test case:

    cleanup
    file mkdir [file join td1 tdx]
    file mkdir [file join td2 tdy]
    testchmod 555 td2
    file copy td1 td3
    file copy td2 td4
    set msg [list [lsort [glob td*]] [glob -directory td3
t*]  [glob -directory td4 t*] [file writable td3] [file
writable td4]]
    if {$tcl_platform(platform) != "macintosh"} {
    testchmod 755 td2
    testchmod 755 td4
    }
    set msg

---- Result was:
{td1 td2 td3 td4} td3/tdx td4/tdy 1 1
---- Result should have been (exact matching):
{td1 td2 td3 td4} td3/tdx td4/tdy 1 0
==== fCmd-10.3 FAILED


==== fCmd-10.8 file rename: comprehensive: dir to new name
and dir FAILED
>>> FAILS BECAUSE the directory permissions are not
preserved on copy.  td4 is writable.  This is how ACL's work
on windows. They are for the most part inherited from the
parent.
==== Contents of test case:

    cleanup
    file mkdir td1
    file mkdir td2
    file mkdir td3
    testchmod 555 td2
    file copy td1 [file join td3 td3]
    file copy td2 [file join td3 td4]
    list [lsort [glob td*]] [lsort [glob -directory td3 t*]]
 [file writable [file join td3 td3]] [file writable [file
join td3 td4]]

---- Result was:
{td1 td2 td3} {td3/td3 td3/td4} 1 1
---- Result should have been (exact matching):
{td1 td2 td3} {td3/td3 td3/td4} 1 0
==== fCmd-10.8 FAILED

==== winFCmd-7.11 TraverseWinTree: call TraversalCopy:
DOTREE_PRED FAILED
>>> FAILS BECAUSE testchmod 000 makes the td1 directory not
writable, and you cannot create td1/tf1
==== Contents of test case:

    cleanup
    file mkdir td1
    testchmod 000 td1
    createfile td1/tf1 tf1
    testfile cpdir td1 td2
    list [file exists td2] [file writable td2]

---- Test generated error; Return code was: 1
---- Return code should have been one of: 0 2
---- errorInfo: couldn't open "td1/tf1": permission denied
    while executing
"open $file w"
    (procedure "createfile" line 2)
    invoked from within
"createfile td1/tf1 tf1"
    ("uplevel" body line 5)
    invoked from within
"uplevel 1 $script"
---- errorCode: POSIX EACCES {permission denied}
==== winFCmd-7.11 FAILED


==== winFCmd-7.19 TraverseWinTree: call TraversalCopy:
DOTREE_POSTD FAILED
>>> FAILS BECAUSE testchmod 000 makes the td1 directory not
writable, and you cannot create td1/tf1
==== Contents of test case:

    cleanup
    file mkdir td1
    testchmod 000 td1
    createfile td1/tf1 tf1
    testfile cpdir td1 td2
    list [file exists td2] [file writable td2]

---- Test generated error; Return code was: 1
---- Return code should have been one of: 0 2
---- errorInfo: couldn't open "td1/tf1": permission denied
    while executing
"open $file w"
    (procedure "createfile" line 2)
    invoked from within
"createfile td1/tf1 tf1"
    ("uplevel" body line 5)
    invoked from within
"uplevel 1 $script"
---- errorCode: POSIX EACCES {permission denied}
==== winFCmd-7.19 FAILED



==== tcltest-8.4 tcltest a.tcl -tmpdir notWriteableDir FAILED
>>> Fails because the {file attributes $notWriteableDir
-readonly 1} command
>>> does not make the directory non writable. Perhaps this
test is invalid
>>> for windows NT as the readonly attribute does not make
the directory
>>> not writeable.
==== Contents of test case:

    slave msg $a -tmpdir $notWriteableDir
    string match {*not writeable*} $msg

---- Result was:
0
---- Result should have been (exact matching):
1
==== tcltest-8.4 FAILED

vincentdarley added on 2006-03-13 05:59:42:
Logged In: YES 
user_id=32170

Donal,

I disagree.  'testchmod' is being used in all these tests to
make a file "not-writable", or "not-readable".  That _is_
it's purpose.  It turns out that it fails in that purpose on
Win2000/XP which uses a more complicated set of APIs to
control readability/writability.

If I understand your thinking correctly, we should make all
'testchmod' tests 'unix only', add a new testfoobar command
which works only on Windows, and then duplicate all the
testchmod tests using this new command and mark them
windows-only.  To me, that's confusion!

It's simple: testchmod is buggy (on XP/2000). Let's fix it.

dkf added on 2006-03-12 06:16:24:
Logged In: YES 
user_id=79902

If testchmod can't test the feature that needs testing, add
another testfoobar command that can. Don't go round trying
to confuse everyone by altering the behaviour of the special
test support commands; life is hard enough as it is without
that sort of confusion added.

nobody added on 2006-03-12 05:59:59:
Logged In: NO 

I don't think we want to remove testchmod -- it has served well till now, but I do 
appreciate that it seems testchmod is a bit buggy. If that code can be updated to 
do things properly, then we'll have a correctly functioning test suite to properly 
evaluate any tclWinFile.c changes.

I also now understand why I was very confused with my testing...

Any chance of a fixed testchmod, then, Francois?

dkf added on 2006-03-11 23:00:00:
Logged In: YES 
user_id=79902

It's not clear to me that testchmod should ever be defined
on Windows (it is based on a POSIX concept after all).
Perhaps a different command would be better there?

jfg118 added on 2006-03-11 10:25:29:
Logged In: YES 
user_id=1266631

There is a revised patch on the on duplicate bug 1344540 to
fix the "read only" files.  

I am still investigating a possible change to the testchmod
command to properly set the ACL to make directories
non-writable since setting the "read only" flag does not
make the directory non writable.

At this point the regression failures seem to be false.

nobody added on 2006-03-11 04:49:10:
Logged In: NO 

I've been looking at the failures, and it appears to be a
bug of testchmod not actully making the directory read only.
 If you do a testchmod 555 you will see that it sets the
"read only" flag on the directory.  However the directory is
writable if you actually try to write something to it.  This
is exactly what this bug refers to...

Example test : 

% file mkdir td3
% testchmod 555 td3
% set f [open td3/tf1 w]
fileadb1d0
% puts $f test
% close $f
% glob td3/tf1
td3/tf1
% file writable td3
1

Without the patch for this bug file writeable returns 0

fvogelnew1 added on 2006-03-11 04:47:20:
Logged In: YES 
user_id=1245417

From Jason's comment below (2005-05-04 01:05):

"
Clearly from the MSDN document it states that the
"Read-only attribute" does not mean that the directory is 
not writable, but instead means it is a "special 
customized folder". The writeability is determined by the 
ACL list.
"

So for instance for the particular test that fails in 
cmdAH.test:

test cmdAH-17.2 {Tcl_FileObjCmd: writable} {
    -constraints {notRoot testchmod}
    -setup   {testchmod 0555 $gorpfile}
    -body    {file writable $gorpfile}
    -result  0
}

$gorpfile has its readonly attribute set (-setup option) 
before running the actual test which is {file writable 
$gorpfile}

But file writable now checks for the ACL list rather than 
for the readonly attribute. This is definitely correct and 
is actually the goal of the patch.

So no surprise if this test fails.

It should just be rewritten to remove write permissions on 
the file instead of setting the readonly attribute before 
trying file writable. Alternatively the test could check 
for -result 1, which really is the expected result for 
file writable regardless of the readonly attribute.

IMHO the test should be updated, not the patch.

I'll have a look at the other failing tests from fCmd.test.

fvogelnew1 added on 2006-03-11 04:22:43:
Logged In: YES 
user_id=1245417

Oups, I found it. Actually the test suite does not run in 
tclsh but in tcltest, that defines the testchmod command.

I said nothing.

fvogelnew1 added on 2006-03-11 04:06:41:
Logged In: YES 
user_id=1245417

nmake -f makefile.vc test TESTFLAGS="-file cmdAH.test" 
sets tcltest::testConstraint testchmod to 1 whereas 
sourcing cmdAH.test in wish or tclsh sets it to 0, making 
the faulty tests to be skipped.

Since I'm currently testing on Windows it should be 0, 
isn't it?

Failing those tests is apparently not due to the patch but 
to info commands testchmod that returns nothing in tclsh 
(correct) while it returns "testchmod" using the makefile 
as if I were on Linux, so that the testchmod constraint 
becomes true. Strange... Any advice?

fvogelnew1 added on 2006-03-11 03:47:23:
Logged In: YES 
user_id=1245417

Hmmm, running the test suite for fCmd.test and cmdAH.test 
using nmake -f makefile.vc test outputs a lot of errors, 
yes.

However, sourcing those two files from wish the following 
way does not:

(tests) 38 % source cmdAH.test
cmdAH.test:Total297Passed115Skipped182
Failed0
Number of tests skipped for each constraint:
4nonPortable
8testchmod
152testsetplatform
1testvolumetype
17unix
(tests) 39 % source fCmd.test
fCmd.test:Total250Passed178Skipped72
Failed0
Number of tests skipped for each constraint:
195
1dontCopyLinks
1nonPortable
3notNetworkFilesystem
17testchmod
1testsetplatform
48unix

How come?

fvogelnew1 added on 2006-03-11 03:12:44:
Logged In: YES 
user_id=1245417

OK, sorry, just tried nmake -f makefile.vc test

Having a look right now.

fvogelnew1 added on 2006-03-11 03:00:30:
Logged In: YES 
user_id=1245417

Do you perhaps have some failing tests log I could have a 
look at?

vincentdarley added on 2006-03-10 17:30:05:
Logged In: YES 
user_id=32170

Unfortunately, cvs HEAD now fails a lot of tests (fCmd.test
and cmdAH.test), so I'm removing the patch.

I'll commit some changes so at least 8-4 and HEAD branches
have the same basic code, but then it will be up to someone
else to understand this ridiculous mess of win32 security
code to come up with the right fix for both.

fvogelnew1 added on 2006-03-10 14:48:39:
Logged In: YES 
user_id=1245417

I confirm that this problem is fixed for me with the cvs 
version, HEAD branch.

tclWinFile.c has not yet been tagged as core-8-4-13 
therefore I can't check.

Francois

dgp added on 2006-03-10 10:04:49:
Logged In: YES 
user_id=80530


someone please confirm this
is fixed for 8.4.13 and
8.5a4 and close this report?

fvogelnew1 added on 2006-03-09 14:17:26:
Logged In: YES 
user_id=1245417

Jason,

I *really* appreciated your help on fixing this.

Many thanks.

Francois

jfg118 added on 2006-03-09 07:26:23:
Logged In: YES 
user_id=1266631

New patch submitted on duplicate bug 1344540

fvogelnew1 added on 2006-03-08 02:22:32:
Logged In: YES 
user_id=1245417

Following Don Porter's advice today about 8.4.13:

> Review, testing, and digging out of any remaining bugs
> that need to
> be fixed before release is invited.  Raise prio to 9.

Thanks,
Francois

jfg118 added on 2005-11-01 12:25:29:
Logged In: YES 
user_id=1266631

Created duplicate bug 1344540

dgp added on 2005-11-01 10:31:01:
Logged In: YES 
user_id=80530


SF Trackers are fundamentally
broken on this point.  See 490389.

Only workaround is to open a new
Tracker report and attach to that.
Please do so.

And if you want to add your voice
of complaint to 490389, it likely won't
matter, but it might make us complainers
feel better. :)

jfg118 added on 2005-11-01 07:25:02:
Logged In: YES 
user_id=1266631

I do not have the option to upload an attachment.  Not sure 
why, perhaps because I did not file the bug?  On bugs I file I 
do have the option to attach a file.

vincentdarley added on 2005-11-01 06:38:49:
Logged In: YES 
user_id=32170

Can you attach your patch as an _attachment_ rather than
pasting it into the comments field (where it is generally
garbled by wrapping, sadly)?

thanks!

jfg118 added on 2005-10-22 06:24:00:
Logged In: YES 
user_id=1266631

Below is a patch which implements the unicode call since 
Tcl seems to use unicode for WinNT by default.  It _corrects_ 
the functionality of the AccessCheck and removes the buffer 
on the stack.  The source is based on the Tcl 8.4.11 source 
and encompasses the changes by dkf. The patch should still 
be easily applied on the CVS HEAD.

The patch is generated with diff -u (3 files)

--- ../src/tcl8.4.11/win/tclWinInt.h2004-05-03 
11:01:36.000000000 -0700
+++ win/tclWinInt.h2005-10-21 11:20:21.390410900 -0700
@@ -76,6 +76,8 @@
     BOOL (WINAPI *getComputerNameProc)(WCHAR *, 
LPDWORD);
     DWORD (WINAPI *getCurrentDirectoryProc)(DWORD, 
WCHAR *);
     DWORD (WINAPI *getFileAttributesProc)(CONST TCHAR 
*);
+    BOOL (WINAPI *getFileSecurityProc)(CONST TCHAR *, 
SECURITY_INFORMATION, 
+            PSECURITY_DESCRIPTOR, DWORD, 
LPDWORD);
     DWORD (WINAPI *getFullPathNameProc)(CONST 
TCHAR *, DWORD nBufferLength, 
     WCHAR *, TCHAR **);
     DWORD (WINAPI *getModuleFileNameProc)(HMODULE, 
WCHAR *, int);
--- ../src/tcl8.4.11/win/tclWin32Dll.c2005-06-06 
14:04:46.000000000 -0700
+++ win/tclWin32Dll.c2005-10-21 
11:20:25.075710100 -0700
@@ -89,6 +89,8 @@
     (BOOL (WINAPI *)(WCHAR *, LPDWORD)) 
GetComputerNameA,
     (DWORD (WINAPI *)(DWORD, WCHAR *)) 
GetCurrentDirectoryA,
     (DWORD (WINAPI *)(CONST TCHAR *)) 
GetFileAttributesA,
+    (BOOL (WINAPI *)(CONST TCHAR *, 
SECURITY_INFORMATION, 
+            PSECURITY_DESCRIPTOR, DWORD, 
LPDWORD)) GetFileSecurityA,
     (DWORD (WINAPI *)(CONST TCHAR *, DWORD 
nBufferLength, WCHAR *, 
     TCHAR **)) GetFullPathNameA,
     (DWORD (WINAPI *)(HMODULE, WCHAR *, int)) 
GetModuleFileNameA,
@@ -138,6 +140,8 @@
     (BOOL (WINAPI *)(WCHAR *, LPDWORD)) 
GetComputerNameW,
     (DWORD (WINAPI *)(DWORD, WCHAR *)) 
GetCurrentDirectoryW,
     (DWORD (WINAPI *)(CONST TCHAR *)) 
GetFileAttributesW,
+    (BOOL (WINAPI *)(CONST TCHAR *, 
SECURITY_INFORMATION, 
+            PSECURITY_DESCRIPTOR, DWORD, 
LPDWORD)) GetFileSecurityW,
     (DWORD (WINAPI *)(CONST TCHAR *, DWORD 
nBufferLength, WCHAR *, 
     TCHAR **)) GetFullPathNameW,
     (DWORD (WINAPI *)(HMODULE, WCHAR *, int)) 
GetModuleFileNameW,
--- ../src/tcl8.4.11/win/tclWinFile.c2005-06-22 
14:23:32.000000000 -0700
+++ win/tclWinFile.c2005-10-21 16:06:21.584662400 -0700
@@ -171,6 +171,7 @@
 static int NativeStat(CONST TCHAR *path, Tcl_StatBuf 
*statPtr, int checkLinks);
 static unsigned short NativeStatMode(DWORD attr, int 
checkLinks, int isExec);
 static int NativeIsExec(CONST TCHAR *path);
+static int NativeIsWritable(CONST TCHAR *path);
 static int NativeReadReparse(CONST TCHAR* 
LinkDirectory, 
      
REPARSE_DATA_BUFFER* buffer);
 static int NativeWriteReparse(CONST TCHAR* 
LinkDirectory, 
@@ -1333,7 +1334,7 @@
 return -1;
     }
 
-    if ((mode & W_OK) && (attr & 
FILE_ATTRIBUTE_READONLY)) {
+    if ((mode & W_OK) && !NativeIsWritable(nativePath)) {
 /*
  * File is not writable.
  */
@@ -1360,6 +1361,102 @@
     return 0;
 }
 
+ /*
+  *----------------------------------------------------------------------
+  *
+  * NativeIsWritable --
+  *
+  *Determine if a path is writable. On Windows this 
requires more than a
+  *simple read only flag check for directories 
because Windows uses this
+  *as a flag to customize the folder options. [Bug 
1193497]
+  *
+  * Results:
+  *1 = writable, 0 = not.
+  *
+  * Side effects:
+  *May smash the last error value (errors are just 
reported by this
+  *function as non-writable files).
+  *
+  *----------------------------------------------------------------------
+  */
+static int
+NativeIsWritable(
+    CONST TCHAR *nativePath)
+{
+    BYTE *secDescPtr = 0;
+    DWORD secDescLen = 0;
+    HANDLE hToken = NULL;
+    BOOL isWritable = FALSE;
+
+#define FILE_SEC_INFO_BITS \
+OWNER_SECURITY_INFORMATION \
+| GROUP_SECURITY_INFORMATION \
+| DACL_SECURITY_INFORMATION
+
+    /*
+     * Read the security descriptor for the file or directory. 
Note the
+     * first call obtains the size of the security descriptor.
+     */
+
+    if (!tclWinProcs->getFileSecurityProc(nativePath, 
FILE_SEC_INFO_BITS,
+NULL, 0, &secDescLen)) {
+        if (GetLastError() == 
ERROR_INSUFFICIENT_BUFFER) {
+            DWORD secDescLen2 = 0;
+    secDescPtr = (BYTE *) ckalloc(secDescLen);
+    if (!tclWinProcs->getFileSecurityProc
(nativePath, FILE_SEC_INFO_BITS,
+        secDescPtr, secDescLen, &secDescLen2) || 
(secDescLen < secDescLen2)) {
+        goto done;
+            }
+        } else {
+    goto done;
+        }
+    }
+
+    /*
+     * Get the security identity of the process and (if 
successful) use it
+     * with the security descriptor of the file to find out if the 
file really
+     * is writable.
+     */
+
+    if (OpenProcessToken(GetCurrentProcess(), 
TOKEN_DUPLICATE, &hToken)) {
+HANDLE hIToken = NULL;
+BOOL status = DuplicateToken(hToken, 
SecurityIdentification, &hIToken);
+
+CloseHandle(hToken);
+if (status) {
+    PRIVILEGE_SET privilegeSet;
+            DWORD privilegeSetLength = sizeof(privilegeSet);
+    DWORD granted; /* unused */
+            DWORD accessDesired = ACCESS_WRITE;
+
+            /* Initialize generic mapping structure to map all. */
+            GENERIC_MAPPING GenericMapping;
+            memset(&GenericMapping, 0xff, sizeof
(GENERIC_MAPPING));
+            GenericMapping.GenericRead = ACCESS_READ;
+            GenericMapping.GenericWrite = ACCESS_WRITE;
+            GenericMapping.GenericExecute = 0;
+            GenericMapping.GenericAll = ACCESS_READ | 
ACCESS_WRITE;
+            MapGenericMask(&accessDesired, 
&GenericMapping);
+
+    AccessCheck(secDescPtr, hIToken, 
accessDesired,
+                        &GenericMapping, &privilegeSet, 
&privilegeSetLength, 
+                        &granted, &isWritable);
+    CloseHandle(hIToken);
+}
+    }
+
+    /*
+     * Release any temporary space we may be using and 
convert the writability
+     * value into a boolean for return.
+     */
+
+  done:
+    if (secDescPtr) {
+ckfree(secDescPtr);
+    }
+    return isWritable != 0;
+}
+
 /*
  *----------------------------------------------------------------------
  *

dkf added on 2005-10-21 15:48:14:

File Added - 153304: writable.patch

dkf added on 2005-10-21 15:48:13:
Logged In: YES 
user_id=79902

Please find attached a patch file that implements the same
changes that Jason suggests, but recut as a UniDiff against
the HEAD (UniDiffs, created with the -u option to cvs diff,
are normally by far the easiest sorts of patches to apply
and review). The changes should apply equally well against
8.4, but I've not checked. I've got the formatting to follow
the core standards, and I've added a few comments to
indicate what *I* think is going on.

I do not know if the patch is correct (I *cannot* review it)
and I wonder whether using 1024 for the statuc buffer size
(as the original patch did) is a bit much. After all, stack
space is comparatively valuable.

jfg118 added on 2005-10-21 06:10:49:
Logged In: YES 
user_id=1266631

missing lines of the patch in the previous comment :

173a174
> static int NativeIsWritable(CONST TCHAR *path);
1336c1337

jfg118 added on 2005-10-21 06:08:44:
Logged In: YES 
user_id=1266631

Below is a patch generated against the Tcl 8.4.11 source.
The specific file is win/tclWinFile.c rev 1.44.2.12 .  The
patch implements and "NativeIsWritable" function.  The
function needs to have the wide char implementation added.
The only call which needs to be updated is the
GetFileSecurity function which needs an entry in the
tclWinProcs defnition.  The patch falls back to the old
implementation for wide char paths. 

<     if ((mode & W_OK) && (attr & FILE_ATTRIBUTE_READONLY)) {
---
>     if ((mode & W_OK) && !NativeIsWritable(nativePath)) {
1365a1367,1442
>  * NativeIsWritable --
>  *
>  *Determines if a path is writable.  On windows this 
>  *requires more than a simple read only flag check for
directories
>  *      because windows uses this as a flag to customize
the folder
>  *      options.  BUG 1193497
>  *
>  * Results:
>  *1 = writable, 0 = not.
>  *
> 
*----------------------------------------------------------------------
>  */
> static int
> NativeIsWritable(nativePath)
>     CONST TCHAR *nativePath;
> {
>   char psd_buf[1024], *psd_ptr = psd_buf, *psd_alloc = 0;
>   DWORD sd_rlen;
>   HANDLE hToken = NULL;
>   int retVal = 0;
> 
>   if (tclWinProcs->useWide) {
>     /* not implemented  -- fall back to old implementation */
>     return
((*tclWinProcs->getFileAttributesProc)(nativePath) &
FILE_ATTRIBUTE_READONLY) ? 0 : 1;
>   }
> 
>   if (!GetFileSecurity(nativePath,
>                        OWNER_SECURITY_INFORMATION
>                        | GROUP_SECURITY_INFORMATION
>                        | DACL_SECURITY_INFORMATION,
>                        psd_buf, sizeof(psd_buf), &sd_rlen)) {
>     return 0; /* Failed ... */
>   }
>   if (sizeof(psd_buf) < sd_rlen) {
>     DWORD psd_alloc_rlen;
>     psd_alloc = (char *)ckalloc(sd_rlen);
>     if (!GetFileSecurity(nativePath,
>                          OWNER_SECURITY_INFORMATION
>                          | GROUP_SECURITY_INFORMATION
>                          | DACL_SECURITY_INFORMATION,
>                          psd_alloc, sd_rlen, &psd_alloc_rlen)
>         || sd_rlen  < psd_alloc_rlen) {
>       ckfree(psd_alloc);
>       return 0; /* Failed .... */
>     }
>     psd_ptr = psd_alloc;
>     sd_rlen = psd_alloc_rlen;
>   }
>   if ( OpenProcessToken ( GetCurrentProcess ( ),
TOKEN_DUPLICATE, &hToken ) ) {
>     HANDLE hIToken = NULL;
>     BOOL status = DuplicateToken (hToken,
SecurityIdentification, &hIToken);
>     CloseHandle (hToken);
>     
>     if (status) {
>       GENERIC_MAPPING mapping = { FILE_GENERIC_READ,
>                                   FILE_GENERIC_WRITE,
>                                   FILE_GENERIC_EXECUTE,
>                                   FILE_ALL_ACCESS };
>       char pbuf[sizeof (PRIVILEGE_SET) + 3 * sizeof
(LUID_AND_ATTRIBUTES)];
>       DWORD plength = sizeof(pbuf), granted; 
>       retVal = AccessCheck (psd_ptr, hIToken,
FILE_WRITE_DATA, &mapping,
>                             (PPRIVILEGE_SET) pbuf,
&plength, &granted, &status) ? 1 : 0;
>       
>       CloseHandle(hIToken);
>     }
>   }
> 
>   if (psd_alloc)
>     ckfree(psd_alloc);
> 
>   return retVal;
> }
> /*
> 
*----------------------------------------------------------------------
>  *

fvogelnew1 added on 2005-10-06 02:47:30:
Logged In: YES 
user_id=1245417

I would be very thankful if somebody could derive a proper 
patch from the code below.
I already had a deep look at this code, but I have to admit 
that I'm not proficient enough to do it myself, sorry for that.

hobbs added on 2005-10-06 02:20:05:
Logged In: YES 
user_id=72656

There is some demo code provided below.  If someone wants to
turn that into a patch for the core, we can try it out.

fvogelnew1 added on 2005-10-05 20:00:09:
Logged In: YES 
user_id=1245417

Hi all,

Anything new wrt this bug?
Is anybody planning to fix it?

I really need [file writable] to work correctly. My scripts are 
currently not portable on Windows because of this bug.

Thanks.

dgp added on 2005-08-24 04:10:24:
Logged In: YES 
user_id=80530

dgpvisitor is upset about [file writable]'s lies on windows.
[17:05]dgp1193497, I think
[17:06]dgphe tracked it down to a fundamental difference
in meaning of permission bits on the two systems.
[17:07]dgpOn unix, when a user lacks write permission to a
directory, the user cannot create/delete files in that
directory.
[17:07]dgpOn Windows, when a users lacks write permission
to a directory, the user cannot delete the directory itself,
but may create/delete files within it as much as they like.

I would prefer to have [file writable] give
consistent information cross-platform, so
I can write portable scripts with it.  Wipe
out OS Differences!

vincentdarley added on 2005-06-09 23:28:06:
Logged In: YES 
user_id=32170

Can you suggest and test a patch to tclWinFile.c which
resolves this for you?

jfg118 added on 2005-05-04 06:09:10:
Logged In: YES 
user_id=1266631

Something like this would check the ACL list for writable
permission, written in C++ though.

bool is_writable(const char *path) {

    char psd_buf[1024], *psd_ptr = psd_buf, *psd_alloc = 0;
    DWORD sd_rlen;

    if (!GetFileSecurity (path,
                          OWNER_SECURITY_INFORMATION
                          | GROUP_SECURITY_INFORMATION
                          | DACL_SECURITY_INFORMATION,
                          psd_buf, sizeof(psd_buf), &sd_rlen)) {
        return false; // Failed ...
    }
    if (sizeof(psd_buf) < sd_rlen) {
        psd_alloc = new char[sd_rlen];
        DWORD psd_alloc_rlen;
        if (!GetFileSecurity (path,
                              OWNER_SECURITY_INFORMATION
                              | GROUP_SECURITY_INFORMATION
                              | DACL_SECURITY_INFORMATION,
                              psd_alloc, sd_rlen,
&psd_alloc_rlen)
            || sd_rlen  < psd_alloc_rlen) {
            delete [] psd_alloc;
            return false; // Failed ....
        }
        psd_ptr = psd_alloc;
        sd_rlen = psd_alloc_rlen;
    }

    HANDLE hToken = NULL, hIToken = NULL;
    if ( !OpenProcessToken ( GetCurrentProcess ( ),
TOKEN_DUPLICATE, &hToken ) ) {
        if (psd_alloc)
            delete [] psd_alloc;
        return false;
    }
    BOOL status = DuplicateToken (hToken,
SecurityIdentification, &hIToken);
    CloseHandle (hToken);

    if (! status) {
        if (psd_alloc)
            delete [] psd_alloc;
        return false;
    }

    GENERIC_MAPPING mapping = { FILE_GENERIC_READ,
                                FILE_GENERIC_WRITE,
                                FILE_GENERIC_EXECUTE,
                                FILE_ALL_ACCESS };
    char pbuf[sizeof (PRIVILEGE_SET) + 3 * sizeof
(LUID_AND_ATTRIBUTES)];
    DWORD plength = sizeof(pbuf), granted;
    if (AccessCheck (psd_ptr, hIToken, FILE_WRITE_DATA,
&mapping,
                     (PPRIVILEGE_SET) pbuf, &plength,
&granted, &status))
        retVal = true;

    CloseHandle(hIToken);

    if (psd_alloc)
        delete [] psd_alloc;

    return true;
}

jfg118 added on 2005-05-04 06:05:33:
Logged In: YES 
user_id=1266631

Clearly from the MSDN document it states that the "Read-only
attribute" does not mean that the directory is not writable,
but instead means it is a "special customized folder". The
writeability is determined by the ACL list.

From MSDN : CAUSE
Unlike the Read-only attribute for a file, the Read-only
attribute for a folder is typically ignored by Windows,
Windows components and accessories, and other programs. For
example, you can delete, rename, and change a folder with
the Read-only attribute by using Windows Explorer. The
Read-only and System attributes is only used by Windows
Explorer to determine whether the folder is a special
folder, such as a system folder that has its view customized
by Windows (for example, My Documents, Favorites, Fonts,
Downloaded Program Files), or a folder that you customized
by using the Customize tab of the folder's Properties dialog
box.

fvogelnew1 added on 2005-05-03 05:11:41:
Logged In: YES 
user_id=1245417

The fact that the read only box cannot be unchecked using 
the Explorer is a Windows by-design item, yes.

But all the directories of my report have this attribute set. 
Nevertheless, [file writable] returns 0 or 1 depending on which 
directory it checks. Moreover, I *can* write files in all of them.

IMO this means that the way tcl implements the file writable 
command should be revised to return 1 for all of them.

Otherwise, how can I check if a directory is writable or not?

In other words, there should be more/other checks than the 
read only attribute (I know there is already more, see bug 
749876, but this is apparently not enough).

hobbs added on 2005-05-03 04:55:32:
Logged In: YES 
user_id=72656

Which would imply that this is a Windows by-design item.

fvogelnew1 added on 2005-05-03 03:52:05:
Logged In: YES 
user_id=1245417

And apparently this is by design of Windows. See 
http://support.microsoft.com/kb/326549/en-us

fvogelnew1 added on 2005-05-03 03:46:21:
Logged In: YES 
user_id=1245417

And I can't remove the read only attribute using Explorer. It 
let me do it, but if I reopen the Properties box, it is here again.

fvogelnew1 added on 2005-05-03 03:25:02:
Logged In: YES 
user_id=1245417

All the directories I mention in my report show the read only 
attribute if I look at their properties using Explorer, 
even "mydir" which I created manually in "Mes Documents".

hobbs added on 2005-05-03 03:07:54:
Logged In: YES 
user_id=72656

While you can actually write to that directory, I think
Windows handles it specially.  If I do through Explorer and
do Properties on my "My Documents" dir, it does clearly have
the "Read only" box checked, which is correctly reported by
Tcl as well.

Attachments: