RFR: 8075810: LogManager.readConfiguration may throw undocumented IllegalArgumentException

[Daniel Fuchs]

Hi,

I received private feedback that using plain IOException
might be preferable to using CharConversionException, as
that doesn't exactly reflect what is going on here.

So here is my new webrev - which includes the minor formatting
issues found by Jason, but reverts to using plain IOException:

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

If I don't hear negative feedback that's what I'm going to
present to CCC.

best regards,

-- daniel

On 25/03/15 12:41, Daniel Fuchs wrote:
> 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>
>>>>
>>>>
>>>>
>>>
>