603 // check that the Class of the Permission key and value are the
same
604 for (Map.Entry<Permission, Permission> e : perms.entrySet()) {
605 Permission k = e.getKey();
606 Permission v = e.getValue();
607 if (!(k.getClass().equals(v.getClass()))) {
608 throw new InvalidObjectException("Permission with " +
609 k.getClass() + " incorrectly mapped to Permission with
" +
610 v.getClass());
611 }
612 }
Why not check k == v? If they were the same object at serialization, then the
deserialization process should be able to assign the same object to them.
--Max
> On Apr 6, 2019, at 1:30 AM, Sean Mullan <[email protected]> wrote:
>
> Updated webrev: http://cr.openjdk.java.net/~mullan/webrevs/8020637/webrev.01/
>
> The serialized streams are now encoded within the test code itself. I also
> added a test case for an PermissionsHash object with invalid mappings.
>
> I also modified the fix. Instead of trying to fix the mappings in the encoded
> serialized form, readObject simply throws an InvalidObjectException. I
> believe that is the more correct behavior, as the deserialized form no longer
> adheres to the invariants of the serialized structures of the corresponding
> classes.
>
> Thanks,
> Sean
>
> On 4/2/19 10:03 AM, Weijun Wang wrote:
>> +1.
>> --Max
>>> On Apr 2, 2019, at 9:55 PM, Roger Riggs <[email protected]> wrote:
>>>
>>> Hi Sean,
>>>
>>> Typically, fixed serialization streams are encoded in the source
>>> as byte arrays. That keeps binary content out of the repo
>>> and provides a place for the comments.
>>>
>>> Roger
>>>
>>>
>>> On 04/02/2019 09:50 AM, Sean Mullan wrote:
>>>> On 4/2/19 9:44 AM, Weijun Wang wrote:
>>>>>
>>>>>
>>>>>> On Apr 2, 2019, at 9:33 PM, Sean Mullan <[email protected]> wrote:
>>>>>>
>>>>>> On 4/1/19 11:12 PM, Weijun Wang wrote:
>>>>>>> I can understand the change in Permissions, but is there any difference
>>>>>>> in PermissionsHash?
>>>>>>
>>>>>> The key and value in the PermissionsHash map is always the same object.
>>>>>> This fix ensures that is respected, otherwise after deserialization you
>>>>>> could have a SocketPermission mapped to a FilePermission, for example.
>>>>>> Would it be better if I added a test for that?
>>>>>
>>>>> Yes, you are right. I thought the old code can also enforce this relation.
>>>>>
>>>>> Now for the test, perms.ser is binary and you haven't described how it is
>>>>> generated.
>>>>
>>>> I just hacked Permissions.writeObject to switch the mappings. That was a
>>>> lot easier than trying to write my own serialization code. I will add some
>>>> comments in the test explaining how I did that and what is in perms.ser.
>>>>
>>>> --Sean
>>>>
>>>>
>>>>>
>>>>> Thanks,
>>>>> Max
>>>>>
>>>>>>
>>>>>> --Sean
>>>>>>
>>>>>>> --Max
>>>>>>>> On Apr 2, 2019, at 1:10 AM, Sean Mullan <[email protected]> wrote:
>>>>>>>>
>>>>>>>> It is currently possible to change the mappings in a serialized
>>>>>>>> java.security.Permissions object such that they no longer map
>>>>>>>> correctly, and Permissions.readObject won't detect this.
>>>>>>>>
>>>>>>>> This change makes sure that for a deserialized Permissions object, the
>>>>>>>> permissions are mapped correctly to the class that they belong to. It
>>>>>>>> does this by calling add() again for each permission in the
>>>>>>>> deserialized Permissions object. The same technique was applied to a
>>>>>>>> serialized PermissionsHash object which is used to store Permissions
>>>>>>>> that don't implement their own PermissionCollection.
>>>>>>>>
>>>>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8020637
>>>>>>>> webrev: http://cr.openjdk.java.net/~mullan/webrevs/8020637/webrev.00/
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Sean
>>>>>>>>
>>>>>
>>>
