RFR (JDK 12/java.base) 8213325: (props) Properties.loadFromXML does not fully comply with the spec

[Joe Wang]

Hi Daniel,

On 11/16/18, 10:32 AM, Daniel Fuchs wrote:
> Hi Joe,
>
> 1. It looks as if the parser will not complain if the root element
>    is seen twice - and it looks as if it is a supported feature
>    since the previous implementation also had a counter.
>
>    Should this be tested?
>    If this is a feature and multiple roots are supported
>    then the new impl might have an issue with allowing only
>    one comment (maybe).

Removed "properties" from the ALLOWED_ELEMENTS so that it will report an 
error on properties inside properties, and added a test 
(IllegalElement.xml).
>
> 2. The rootElm qName is now set at two locations. Is the overridden
>    method below still needed? Could it have been used instead of
>    introducing a new startDTD method?
>
>  212     @Override
>  213     public void notationDecl(String name, String publicId, String 
> systemId) throws SAXException {
>  214         rootElm = name;
>  215     }

Removed the method override. startDTD is the correct place to accomplish 
this.

Updated webrev:
http://cr.openjdk.java.net/~joehw/jdk12/8213325/webrev_v03/

Previous:
http://cr.openjdk.java.net/~joehw/jdk12/8213325/webrev_v02/

Best regards,
Joe

>
> best regards,
>
> -- daniel
>
> On 15/11/2018 20:26, Joe Wang wrote:
>> Hi Daniel,
>>
>> I deleted the endDTD method. It could have been used to signal the 
>> end of DTD parsing and therefore serve as a validation point. 
>> However, the parser would have thrown Exceptions if a DTD parsing 
>> wasn't completed properly, that seems to make an endDTD check 
>> unnecessary, at least as the current parser impl goes.
>>
>> The small footprint parser has its problems, for example, not 
>> providing specific error messages when things go wrong. But since 
>> this is a Properties usage only, it serves the purpose well enough.
>>
>> Updated webrev:
>> http://cr.openjdk.java.net/~joehw/jdk12/8213325/webrev_v02/
>>
>> Previous:
>> http://cr.openjdk.java.net/~joehw/jdk12/8213325/webrev_v01/
>>
>> Best regards,
>> Joe
>>
>> On 11/14/18, 2:32 AM, Daniel Fuchs wrote:
>>> Hi Joe,
>>>
>>> I do not see where the new DTDHandler::endDTD methos is called.
>>> Is that an oversight or is there more magic?
>>>
>>> best regards,
>>>
>>> -- daniel
>>>
>>> On 14/11/2018 05:18, Joe Wang wrote:
>>>> Hi,
>>>>
>>>> Please review a patch to bring the small footprint parser for 
>>>> java.util.Properties compliant with the specification.
>>>>
>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8213325
>>>> webrevs: http://cr.openjdk.java.net/~joehw/jdk12/8213325/webrev/
>>>>
>>>> Thanks,
>>>> Joe
>>>
>