On 03/25/2015 04:58 PM, Chris Hegarty wrote:
On 25 Mar 2015, at 17:32, Peter Levart <[email protected]> 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
