{PING!} [9] RFR JDK-8068373: (prefs) FileSystemPreferences writes \0 to XML storage, causing loss of all preferences

[Brian Burkhalter]

On Feb 18, 2015, at 7:27 AM, Brian Burkhalter <brian.burkhalter at oracle.com> wrote:

> On Feb 18, 2015, at 12:47 AM, Alan Bateman <Alan.Bateman at oracle.com> wrote:
> 
>> On 17/02/2015 21:56, Brian Burkhalter wrote:
>>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-February/031502.html
>>> 
>>> 
>> It's good to see this changed to throw IAE. What about other methods that take a key (like get), do they need their javadoc updated too?
> 
> That’s a good point. I’ll take a look while the push of this change is pending the outcome of the CCC request I submitted.

Well the CCC request for this change to the put() method has been approved so now I return to the topic addressing the point above and noting an alternate possible approach.

1) Current Approach

If the present approach is maintained, the get(String,String) and remove(String) methods should also throw IAEs if any of their parameters are code point U+0000. This would require an updated patch and a re-spin of the review and CCC processes.

2) Alternate Approach

Reviewing the documentation in j.u.prefs.AbstractPreferences suggested to me an alternate approach perhaps more consistent with the extant specification. To be specific, consider these excerpts from the class documentation of AbstractPreferences:

<classDoc>
A. The SPI methods fall into three groups concerning exception behavior. The getSpi method should never throw exceptions, […]

B. The remaining SPI methods putSpi(String,String), removeSpi(String) and childSpi(String) have more complicated exception behavior. They are not specified to throw BackingStoreException, as they can generally obey their contracts even if the backing store is unavailable. This is true because they return no information and their effects are not required to become permanent until a subsequent call to Preferences.flush() or Preferences.sync(). Generally speaking, these SPI methods should not throw exceptions. In some implementations, there may be circumstances under which these calls cannot even enqueue the requested operation for later processing. Even under these circumstances it is generally better to simply ignore the invocation and return, rather than throwing an exception. Under these circumstances, however, all subsequent invocations of flush() and sync should return false, as returning true would imply that all previous operations had successfully been made permanent.
</classDoc>

Note that the last sentence of the second paragraph above is in fact incorrect as both flush() and sync() are declared to “return” void. Another bug ...
 
Considering the foregoing it seems to make some sense to make the following changes instead of those suggested in the currently approved approach:

i. getSpi(‘\u0000’), putSpi(‘\u0000’, ‘\u0000’) and removeSpi(‘\u0000’) are no-ops and throw no exception
ii. childSpi(U+0000) does something TBD

At your convenience any comments  you might be able to provide on these alternatives would be appreciated.

Thanks,

Brian