Request for review: 7190897 (fs) Files.isWritable method returns false when the path is writable (win). - ver. 1

[Alexey Utkin]

On 04.03.2013 15:51, Alan Bateman wrote:
> On 04/03/2013 09:31, Alexey Utkin wrote:
>> Alan,
>> Do you agree with
>>>     /**
>>>      * Check the access right against the securityInfo in the 
>>> current thread.
>>>      */
>>>     static boolean checkAccessMask(long securityInfo, int accessMask,
>>>         int genericRead, int genericWrite, int genericExecute, int 
>>> genericAll)
>>>         throws WindowsException
>>>     {
>>>         int privilegies = TOKEN_QUERY;
>>>         long hToken = OpenThreadToken(GetCurrentThread(), 
>>> privilegies, false);
>>>         if (hToken == 0L && processTokenWithDuplicateAccess != 0L)
>>>             hToken = DuplicateTokenEx(processTokenWithDuplicateAccess,
>>>                 privilegies);
>>>
We can have the situation with:
1) OpenThreadToken return 0 without exception - that is ok for thread 
without impersonation.
2) processTokenWithDuplicateAccess  is 0 for some reason (weak precess 
privileges)
For that case we have no access to process token without exception 
(hToken :=: 0).
>>>         boolean hasRight = false;
>>>         if (hToken != 0L) {
So, in upper line we need to check token for non-empty value.
! Do you concern about the [false] return value for that case?
>>>             try {
>>>                 hasRight = AccessCheck(hToken, securityInfo, 
>>> accessMask,
>>>                     genericRead, genericWrite, genericExecute, 
>>> genericAll);
Here is the actual work, that can make an exception (for example it 
happens for invalid SID)
>>>             } finally {
>>>                 CloseHandle(hToken);
If token was open it have to be closed without excuses.
>>>             }
>>>         }
>>>         return hasRight;
>>>     }
>> implementation approach?
> Can the handle to the token (hToken) be 0? In my comment I was 
> suggesting:
>
> long hToken = OpenThreadToken(...);
> try {
>     ...
> } finally {
>     CloseHandle(hToken);
> }

> but that doesn't work if OpenThreadToken succeeds with a handle of 0. 
> If 0 is not possible then there are a few other clean-ups that we 
> could do at a later time.
>> Ah, I remember now, we handle ERROR_NO_TOKEN so this means that 
>> hToken can be 0. So this means that the CloseHandle does need to 
>> check for 0 in the finally block.
>>
>> -Alan. 
>
> -Alan
>