Explicit Serialization API and Security

David M. Lloyd david.lloyd at redhat.com
Wed Jan 7 16:14:58 UTC 2015


On 01/07/2015 10:02 AM, Peter Levart wrote:
> On 01/07/2015 03:54 PM, Chris Hegarty wrote:
>> On 06/01/15 17:49, Peter Levart wrote:
>>> On 01/06/2015 06:21 PM, Chris Hegarty wrote:
>>>> On 6 Jan 2015, at 15:06, Peter Levart <peter.levart at gmail.com> wrote:
>>>>
>>>>> On 01/06/2015 04:03 PM, Peter Levart wrote:
>>>>>> private void readObject(ObjectInputStream in) throws IOException,
>>>>>> ClassNotFoundException {
>>>>>>         ObjectInputStream.GetField fields = in.readFields(); // this
>>>>>> already validates the types
>>>>> Well, not true currently. But type validation could be added at this
>>>>> point.
>>>> Right. I think I’ll file a bug to track this as it seems reasonable to
>>>> add type validation to readFields and defaultReadObject. So we can
>>>> probably assume/ignore it in this discussion.
>>>>
>>>> I like the idea of a callback into the serialization framework to
>>>> handling the setting of final fields, after validation. I played a
>>>> little with your patch and added it to a branch in the sandbox**
>>>>
>>>> So a simple example, without legacy fields, might looks as below (
>>>> without the need for writeObject or serialPersistentFields ). The
>>>> simple validating readObject is starting to look like boilerplate ?
>>>
>>> Well, 1st and last line are always the same, yes. What's between them is
>>> what you would have to write in a special check-only method too.
>>
>> I guess what I'm getting at is, if you want just invariant checking,
>> then maybe something like this would be more readable:
>>
>>     @SerialInvariantChecker()
>>     private static void validate(@SerialParam(name="lo",
>> type=Integer.class)int lo,
>>                                  @SerialParam(name="hi",
>> type=Integer.class)int hi)
>>     {
>>         if (lo > hi)
>>             throw new IllegalArgumentException("lo:" + lo + " > hi:" +
>> hi);
>>     }
>>
>>    ... and the serialization machinery would call this when appropriate.
>
> Nice.
>
> We have method parameter names accessible via reflection since JDK8 and
> types have always been, so no @SerialParam is necessary.

I tend to disagree, a bit.  Most superficially, there's no reason to 
bind serialization stream parameter names to method parameter names. 
But more importantly, this implies a pretty complex implementation when 
you consider we already have an API to represent the fields of a class 
in a stream (GetFields) as well as a well-established convention of 
using specifically-named private members for this kind of thing (in the 
form of readObject/writeObject, objectStreamFields, etc.).

You could just as well support this approach, and probably with a 
substantially simpler implementation:

   private static void validate(GetField fields) {
      if (fields.getInt("lo") > fields.getInt("hi")) { ... }
   }

But given these principles, I think this idea proposal was superior and 
is a sort of obvious "why wasn't it always this way" kind of thing:

>>>On 01/06/2015 06:21 PM, Chris Hegarty wrote:
>>>>[...]
>>>>      private void readObject(ObjectInputStream in) throws IOException,
>>>> ClassNotFoundException {
>>>>          ObjectInputStream.GetField fields = in.readFields();
>>>>
>>>>          // validate 'lo' and 'hi' fields invariant
>>>>          int lo = fields.get("lo", 0);
>>>>          int hi = fields.get("hi", 0);
>>>>          validate(lo, hi);
>>>>
>>>>          // set current fields from read data
>>>>          fields.defaultReadFields(); // this is new API!
>>>>      }

-- 
- DML



More information about the core-libs-dev mailing list