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
