RFR (JDK 12/java.base) 8213325: (props) Properties.loadFromXML does not fully comply with the spec
Daniel Fuchs
daniel.fuchs at oracle.com
Fri Nov 16 18:32:43 UTC 2018
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).
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 }
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
>>
More information about the core-libs-dev
mailing list