On 02/11/2011 19:59, David Holmes wrote:
I'm not seeing a distinction in those two statements. Both expect to
return the property value for a given key; both assume a valid value
is a String. clearProperty throws ClassCastException if the assumption
doesn't hold; getProperty instead returns null.
The distinction is "value" vs. "string value", where the latter is
interpreted by Darryl's patch to be the String representation. While
that avoids the CCE, it is clearly inconsistent with System.getProperty
that I see is also specified to return the "string value". So I think we
have to dismiss this approach. I think this leaves two options:
1. Document existing behavior, meaning @throws CCE (several of
Properties methods already have this).
2. Change the method so that it is consistent with getProperty and so
returns null if the property value is not a String.
While option 2 is clearly a change then I think it's worth exploring to
understand the impact. Properties has always had a note in its javadoc
to warn users that it inherits from Hashtable and it has a note to
"strongly discourage" inserting entries that have a key or value that is
not a String. My guess (and this is purely a guess) is that
System.clearProperty is not used very often and so the likelihood of
someone abusing the system properties and using clearProperty is
probably low. I see the bug on bugs.sun.com doesn't have any votes or
comments which suggests that this one doesn't have an angry mob either.
Darryl - since you have decided to tackle this one then I would suggest
looking into this further and coming back with a recommendation. I would
also suggest that as part of the patch that the javadoc for
System.getProperty and Properties.getProperty be clarified so that it's
clear that they return null when the properties have been compromised.
-Alan.
