Hi Darryl,
On 2/11/2011 3:51 AM, Darryl Mocek wrote:
Hello. Please review this patch to fix Bug #6578042
(System.clearProperty() throws ClassCastException if property value
isn't a String). The issue is that through the Hashtable methods, it's
possible to add a Property which isn't a String and set it through
System.setProperties. clearProperty cast the returned removed object as
a String, even if it wasn't a String. Test case provided.
Webrev: http://cr.openjdk.java.net/~sherman/darryl/6578042/webrev
This fix seems inconsistent with how this non-String problem is handled
elsewhere. Eg Properties itself does:
public String getProperty(String key) {
Object oval = super.get(key);
String sval = (oval instanceof String) ? (String)oval : null;
return ((sval == null) && (defaults != null)) ?
defaults.getProperty(key) : sval;
}
This will only return actual String values or else null - it won't
convert non-String values to Strings.
Seems to me that Properties should override remove(Object key) to
perform similar logic as getProperty(String key). That would fix the
System.clearProperty issue.
Or, if you want to confine the change to System do:
synchronized(props) {
String ret = props.getProperty(key);
props.remove(key);
return ret;
}
Not quite as efficient of course.
That said, I'd also support a spec change for clearProperty that states
that if the property has been set to a non-String value then
ClassCastException is thrown. I'm somewhat bemused that Properties
didn't override the necessary Hashtable methods to enforce Strings-only
in the first place.
Cheers,
David Holmes
