RFR: 8075810: LogManager.readConfiguration may throw undocumented IllegalArgumentException

[Daniel Fuchs]

Thanks for the review Jason!

On 24/03/15 18:01, Jason Mehrens wrote:
> Hi Daniel,
>
> Looks good.  The only other alternative would be to use java.io.CharConversionException over IOException.  We could even consider dropping the cause because the subclass of I/O exception would convey the same meaning.

Here is a an updated webrev where I use
java.io.CharConversionException instead of plain IOException.

http://cr.openjdk.java.net/~dfuchs/webrev_8075810/webrev.01/

Note: I had a look in the JDK sources and CharConversionException
doesn't appear to be widely used. Is this the better alternative?
I'd be happy with both (webrev.01 or webrev.00).
Using CharConversionException means that we have to trust that
Properties.load will obey its spec and only throw IAE in case
of character conversion issues.

> Minor formatting issues with a missing space after the catch keyword

Done. Thanks for catching it.

> and missing a tab ahead of 'props.load'

That is an artifact of how webrev calls diff I think.
If you look at the frames version you will see that the tab is here.

best regards,

-- daniel

>
> Jason
>
>
> ----------------------------------------
>> Date: Tue, 24 Mar 2015 16:06:47 +0100
>> From: daniel.fuchs at oracle.com
>> To: lance.andersen at oracle.com
>> Subject: Re: RFR: 8075810: LogManager.readConfiguration may throw undocumented IllegalArgumentException
>> CC: core-libs-dev at openjdk.java.net
>>
>> Hi Lance,
>>
>> Thanks for the review!
>>
>> On 24/03/15 16:01, Lance Andersen wrote:
>>> Hi Daniel,
>>>
>>> This looks OK but I might suggest clarifying that the cause could be set
>>> in the javadoc. Now that being said, I am not sure how consistent we
>>> are across the JDK or just make the assumption people will check the
>>> cause if they desire.
>>
>> Hmmm. I have the feeling that the best place for that would be i the
>> release notes - rather than in the API doc (which reminds me I should
>> plan to add some release note text to the issue).
>>
>>> As far as backporting, unless it really needed, I would probably would
>>> not as a change such as this typically should be done as part of an
>>> errata/MR (due to change of behavior and it is not that big of an issue).
>>
>> Right, my thinking too. Thanks for sharing your opinion!
>>
>> best regards,
>>
>> -- daniel
>>
>>>
>>> Best
>>> Lance
>>> On Mar 24, 2015, at 10:42 AM, Daniel Fuchs <daniel.fuchs at oracle.com
>>> <mailto:daniel.fuchs at oracle.com>> wrote:
>>>
>>>> Hi,
>>>>
>>>> Please find below a fix for
>>>>
>>>> 8075810: LogManager.readConfiguration may throw
>>>> undocumented IllegalArgumentException
>>>>
>>>> https://bugs.openjdk.java.net/browse/JDK-8075810
>>>>
>>>> The proposed fix is to catch the IllegalArgumentException and
>>>> wrap it in an IOException, since LogManager.readConfiguration
>>>> is specified to throw IOException "if there are problems reading
>>>> from the stream."
>>>>
>>>> http://cr.openjdk.java.net/~dfuchs/webrev_8075810/webrev.00/
>>>>
>>>> Initial discussion around the issue can be seen here:
>>>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-March/032348.html
>>>>
>>>> Question for reviewers: I will log a CCC for this - but I am wondering
>>>> whether I should plan to backport the fix to earlier release?
>>>>
>>>>
>>>> best regards,
>>>>
>>>> -- daniel
>>>
>>> <http://oracle.com/us/design/oracle-email-sig-198324.gif>
>>> <http://oracle.com/us/design/oracle-email-sig-198324.gif><http://oracle.com/us/design/oracle-email-sig-198324.gif>
>>> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen|
>>> Principal Member of Technical Staff | +1.781.442.2037
>>> Oracle Java Engineering
>>> 1 Network Drive
>>> Burlington, MA 01803
>>> Lance.Andersen at oracle.com <mailto:Lance.Andersen at oracle.com>
>>>
>>>
>>>
>>