RFR (S) 8230407 : SocketPermission and FilePermission action list allows leading comma

[Ivan Gerasimov]

Thank you Joe for checking it!

On 10/2/19 4:38 PM, Joe Darcy wrote:
> Hello,
>
> At least from a quick reading, either the spec change or the behavior 
> change would seem to merit a CSR.
>
Sigh.  I was hopping it'll be a quick fix :-)

So, I filed CSR: https://bugs.openjdk.java.net/browse/JDK-8231805 to 
cover the addition of @throws paragraph in the javadoc of SocketPermission.

I would really appreciate it, if someone helped to review it.

W.r.t the behavior change, I don't think the fix has to be counted as 
such.   Current implementation already would throw 
IllegalArgumentException if the action list were malformed (for example 
if it were " , accept", or "connect,,accept", or "connect,", etc.)  The 
only case when it would *not* throw IAE is when the argument 
*immediately* starts with a comma, and that's what the fix is about.

It's not like if we used to allow commas in arbitrary places and stopped 
doing that.  Instead, it just turned out that the code fails to catch 
one specific pattern of malformed action list.

With kind regards,

Ivan


> Cheers,
>
> -Joe
>
> On 10/2/2019 4:26 PM, Ivan Gerasimov wrote:
>> Hi Chris!
>>
>> Thank you very much for review!
>>
>> I agree that it makes sense to update the javadoc for consistency.
>>
>> I don't think CSR is required in this case, is it? (IAE is unchecked 
>> anyway, and the fix doesn't really change the behavior.)
>>
>> Here's the updated webrev:
>>
>> http://cr.openjdk.java.net/~igerasim/8230407/01/webrev/
>>
>> With kind regards,
>>
>> Ivan
>>
>>
>> On 10/2/19 6:44 AM, Chris Hegarty wrote:
>>> Ivan,
>>>
>>> On 01/10/2019 21:26, Ivan Gerasimov wrote:
>>>> Hello!
>>>>
>>>> The constructors of SocketPermission and FilePermission expect a 
>>>> String argument with comma-separated list of actions.
>>>>
>>>> If the list is malformed, then the constructors throw 
>>>> IllegalArgumentException.
>>>>
>>>> It turns out that the current implementation fails to throw IAE if 
>>>> the list starts with a leading comma.
>>>>
>>>> Would you please help review a simple fix, which will make the 
>>>> behavior more consistent?
>>>>
>>>> BUGURL: https://bugs.openjdk.java.net/browse/JDK-8230407
>>>> WEBREV: http://cr.openjdk.java.net/~igerasim/8230407/00/webrev/
>>>
>>> The implementation changes look ok.
>>>
>>> The SocketPermission constructor should be updated to specify IAE 
>>> too, right?
>>>
>>> -Chris.
>>>
>>>
>
-- 
With kind regards,
Ivan Gerasimov