<Swing Dev> [10] Review request for 8182041: File Chooser Shortcut Panel folders under on JDK 9

Semyon Sadetsky semyon.sadetsky at oracle.com
Mon Dec 18 22:26:56 UTC 2017


Hi Phil,

On 12/06/2017 09:56 AM, Phil Race wrote:

>
>
> On 12/06/2017 09:39 AM, Semyon Sadetsky wrote:
>>
>> On 12/06/2017 08:33 AM, Phil Race wrote:
>>
>>> Hi,
>>> I have some additional comments on this old review thread. Hopefully 
>>> we can close it out soon
>>> although it will need a CSR whatever ..
>>>
>>> Since the implementation of ShellFolderManager filters out 
>>> inaccessible files we
>>> should document this somewhere.
>>> I suggest either on the class or relevant methods saying something like
>>> "Files or resources which are not accessible in the current security 
>>> context
>>> may be filtered out from the returned set".
>>>
>>> The word "may" is key here ..
>> This looks to me like implementation details.
>
> No. On the contrary. It can be specified.
You are suggesting to describe some solution as a part of a generic 
interface specification which assumes platform specific behavior. But if 
we add this extra details we need to be sure that the proposed generic 
solution is feasible.  At the moment there are no way to set permissions 
for virtual shell folders like "Network" or "Computer" and others but 
they are returned in the list. So, when in  the interface specification 
you are requesting inaccessible elements not to be returned you need to 
be sure that at least the current OpenJDK implementation is in alignment 
with it.
>
>> I agree that it is worth to mention this problematic in the method 
>> spec but since it is a generic method for different platforms it 
>> probably should be given in a different form than a particular 
>> platform specific solution. If the entries of the list are virtual 
>> they may not have any file system permissions at all.
>>>
>>> If we are sure that this is always the case then it would follow 
>>> that SecurityException
>>> does not need to be documented.
>>> Consistency would suggest that then this policy would extend to the 
>>> other methods
>>> added in JDK 9 which declare that exception. So all or none.
>>> Being a RuntimeException that is not checked I think we can 
>>> compatibly remove those.
>> This is not consistent with other JDK classes in which the 
>> SecurityException is always mentioned, for example, the 
>> java.awt.Desktop class and there are many others. I think it would be 
>> non-practical to rewrite all other specs because you've changed your 
>> mind in this particular fix review.
>
> The API contract of some other class such as Desktop is not relevant.
>
> I am saying FileSystemView should be internally consistent
I think more important criterion is the method specification is 
consistent with its own implementation and describes its results in full 
for developer. I have several bugs on my plate which suggest that it is 
important to remind about security exception which may be the result of 
the method otherwise the functionality of the client classes may be broken:
https://bugs.openjdk.java.net/browse/JDK-8193766
https://bugs.openjdk.java.net/browse/JDK-8193563
https://bugs.openjdk.java.net/browse/JDK-8193761

Most of the method specifications of the FileSystemView class *should* 
mention SecurityException as a result of their execution. For example, 
similar java.awt.Desktop class methods which provide access to the OS 
shell extensions all mention SecurityException.

--Semyon
>
>
>> Also, in this fix review [1]  you brought an opposite point of view.
>>
>> [1] 
>> http://mail.openjdk.java.net/pipermail/swing-dev/2015-October/005073.html
>
> That was because no one told me at the time that we were filtering out 
> to prevent leakage.
> i.e I have new information.
>
>
> -phil.
>>
>> --Semyon
>>
>>>
>>> -phil.
>>>
>>>
>>> On 10/24/2017 09:22 PM, Alexander Zvegintsev wrote:
>>>>
>>>> Hi Semyon,
>>>>
>>>> the fix looks good to me, but I found a minor typo in the test:
>>>>
>>>> testShortcatPanelFiles -> testShortcutPanelFiles
>>>>
>>>> no need for a new webrev
>>>>
>>>> Thanks,
>>>> Alexander.
>>>> On 04/10/2017 00:41, Semyon Sadetsky wrote:
>>>>> Hello,
>>>>>
>>>>> Please review fix for JDK10 (the changes involve AWT and Swing):
>>>>>
>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8182041
>>>>>
>>>>> webrev: http://cr.openjdk.java.net/~ssadetsky/8182041/webrev.00/
>>>>>
>>>>> New API method was added letting to query shortcut panel entries 
>>>>> for JFileChooser.
>>>>>
>>>>> --Semyon
>>>>>
>>>>
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/swing-dev/attachments/20171218/0bae4665/attachment.html>


More information about the swing-dev mailing list