RFR [9] 8071472: Add field setter API for setting final fields in readObject

[David M. Lloyd]

On 03/25/2015 04:58 PM, Chris Hegarty wrote:
>
>> On 25 Mar 2015, at 17:32, Peter Levart <peter.levart at gmail.com> wrote:
>>
>> On 03/25/2015 05:55 PM, Peter Levart wrote:
>>> On 03/25/2015 04:49 PM, Chris Hegarty wrote:
>>>>> I have been thinking about this and I see several solutions:
>>>>>
>>>>> 1. provide protected final methods
>>>>> ObjectInputStream.checkPersistentFinalsNotSet() and
>>>>> markPersistentFinalsSet() that just delegate to FieldSetterContext for
>>>>> ObjectInputStream classes to call as part of their own overriden
>>>>> defaultReadObject() method. We should make sure those methods are called
>>>>> in JDK's corba InputStreamHook and document they should be called in 3rd
>>>>> party subclasses.
>>>>
>>>> Yes, I think I agree with this, but since the FieldSetterContext is constructed in OIS private readSerialData, there is nothing to delegate to, unless InputStreamHook/IIOPInputStream creates the context.
>>>
>>> Ah, I see. Let me look at this in some more detail...
>>>
>>> Regards, Peter
>>>
>>
>> Huh, this is tricky! IIOPInputStream does invoke readObject/writeObject methods on an object being (de)serialized, but it does it on it's own. So any classes using FieldSetter API will get an exception when attempted deserialization with corba…
>
> It will get NotActiveException, thinking that it is not in a readObject callback <sigh>.
>
>> We could fix IIOPInputStream to do the right thing at the right time (like ObjectInputStream) and provide a FieldSetter instance, but what about any 3rd party ObjectInputStream subclasses that might do the same? Suddenly classes using FieldSetter API will become incompatible with 3rd party OIS implementations until those implementations catch-up.
>
> Yeap.
>
>> Providing a factory for FieldSetter instance to subclasses of OIS is giving write access to private fields of any object. Would have to be protected by some permission. Is this acceptable?
>
> I think it needs to be protected by more than a permission check. The nice thing about the FieldSetter API as it stands ( before this ) is that it is only possible to set final fields during deserialization. I am reluctant about exposing this API further. Whatever changes we make should keep this property.
>
> I’m looking at your previous suggestions, and giving this some thought. Nothing jumping out yet.

I think this is a sign that this extra check isn't really appropriate, 
or maybe isn't in the appropriate place.  Having a run time check here 
is just "weird", for lack of a better word, given that the equivalent 
constructor check is compile-time.  The author of a class can already 
determine whether they are setting all the fields they want to set; it's 
not like there's generally going to be a confluence of multiple players 
contributing to the set of final fields, or some other complex situation 
like that.

Put another way - the FieldSetter API doesn't *need* this extra check in 
order to justify its existence, especially given these complications. 
If we're just looking for something to replace using reflection to hack 
in final field values, then just having a proper API to do it is already 
enough of an improvement in my mind that I believe that the extra check 
can (and should) be considered as a separate enhancement on top of it, 
if it *must* be considered at all.

-- 
- DML