8004371: (props) Properties.loadFromXML needs small footprint XML parser as fallback when JAXP is not present

[Mandy Chung]

On 12/7/12 3:24 PM, Joe Wang wrote:
> Thanks for reviewing the changes.  Please see comments inline.
>
> On 12/7/2012 11:38 AM, Mandy Chung wrote:
>> On 12/5/12 10:58 AM, Alan Bateman wrote:
>>> http://cr.openjdk.java.net/~alanb/8004371/webrev/
>>>
>>
>> Yay!  Properties no longer requires JAXP to be present in order to 
>> load/store properties in XML format.  This looks okay to me.  Some 
>> minor comments:
>>
>> XMLStreamWriterImpl.isEncodingSupported - it returns true if any 
>> string != "UTF-32" that assumes the given encoding is one of the few 
>> known values.  Would it be better to check the explicit list of 
>> supported encoding?
>>     private boolean isEncodingSupported(String encoding) {
>>         if (encoding.equalsIgnoreCase("UTF-32")) {
>>             return false;
>>         }
>>         return true;
>>     }
>
> The spec only requires UTF-8 and 16. But the writer can actually 
> handle most of the encodings. An explicit list would be quite long and 
> require comprehensive tests. Since the ukit parser explicitly rejects 
> UTF-32, I chose to do the same.  Other than that, the XMLWriter leaves 
> it to java.nio.charset.Charset to determine if a specified encoding is 
> supported through this call:
>        try {
>             cs = Charset.forName(encoding);
>         } catch (IllegalCharsetNameException | 
> UnsupportedCharsetException ex) {
>             throw new XMLStreamException(new 
> UnsupportedEncodingException(encoding));
>         }

OK - you leave it for XMLWriter to check if it's a valid encoding.  It 
may be better to include Charset.forName check in the 
isEncodingSupporting method that would avoid confusion.

>>
>> PropertiesDefaultHandler.java L115-116: can be replaced with 
>> Properties.stringPropertyNames() and foreach.
>
> Done. Will send update to Alan.
>
>>
>> XMLStreamWriterImpl.java L156 - since the caller needs to unwrap 
>> XMLStreamException to determine if the encoding is supported or not, 
>> maybe better to throw this constructor to throw 
>> UnsupportedEncodingException.
>
> The writer implements partially the original StAX API.  So I tried not 
> to change the writer API. But java.util.Properties expect 
> UnsupportedEncodingException in case of invalid encoding, thus the 
> compromise.
>
OK - PropertiesDefaultHandler uses the XMLStreamWriter API.  That's fine 
with me.

Thanks
Mandy