On 4/6/19 10:33 AM, Weijun Wang wrote:
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.
I agree, good catch. I have changed the above block of code to:
603 // check that the Permission key and value are the same object
604 for (Map.Entry<Permission, Permission> e : perms.entrySet()) {
605 Permission k = e.getKey();
606 Permission v = e.getValue();
607 if (k != v) {
608 throw new InvalidObjectException("Permission (" + k +
609 ") incorrectly mapped to Permission (" + v + ")");
610 }
611 }
If you are ok with that, I will push the fix.
--Sean
--Max
On Apr 6, 2019, at 1:30 AM, Sean Mullan <sean.mul...@oracle.com> 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 <roger.ri...@oracle.com> 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 <sean.mul...@oracle.com> 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 <sean.mul...@oracle.com> 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
