Am 05.12.2011 12:53, schrieb Weijun Wang:
On 12/03/2011 06:12 AM, Stuart Marks wrote:
I'm adding Weijun (Max) Wang to this thread.
The same "ackbarfaccept" code had come up a third time when I was
reviewing some of Max's changes. The code in question all has to do with
permissions, and Max is in the security group, so he might have a better
insight whether doing a refactoring is appropriate and how to approach
doing it.
Maybe we can group all permissions with a target and a predefined set
of actions under a new abstract class.
This sounds like the best idea from the implementors point of view. I
would prepare a patch as suggestion for this.
Regarding to my own question i would say that there isn't any additional
jigsaw related problem here by
introducing a common abstract class. They all rely on
java.security.Permission, so java.security seems the correct
package for the new abstract class.
Any suggestions for the name? PermissionWithActions???? AbstractPermission??
If i see it right there is the following class hierarchy:
j.s.Permission implements Serializable
+j.i.FilePermission
+j.n.SocketPermission
+jx.m.MBeanPermission
+jx.s.a.k.ServicePermission
+j.s.BasicPermission
+j.u.PropertyPermission
What happens to serialization when the new hierarchy would look like this:
j.s.Permission implements Serializable
+j.s.AbstractPermission
+j.i.FilePermission
+j.n.SocketPermission
+jx.m.MBeanPermission
+jx.s.a.k.ServicePermission
+j.s.BasicPermission
+j.u.PropertyPermission
All the classes have a serialVerionUID so i think serialisation is no
problem, or is there
a problem i i don't see here?
-- Sebastian
-Max
Some searching around reveals that "ackbarfaccept" appears in the
PlayerPermission class of the Java ME Mobile Media API (not in any of
the OpenJDK repositories) so it looks like the code has been copied even
more times than are visible here. Perhaps a public permissions parsing
utility method is called for.
s'marks
On 12/2/11 9:02 AM, Sebastian Sickelmann wrote:
Am 02.12.2011 16:27, schrieb Brandon Passanisi:
Hi Sebastian. I'm not sure if you had seen the e-mail from Stuart
Marks
regarding this, but Stuart was able to find more instances of the
similar
block of "fallthrough" code. I can volunteer to apply your upcoming
change
to FilePermission to the other files if you wish. Or, you can try
applying
the change to the other files, but if you don't have time I don't
mind doing
it. Here's the section of Stuart's e-mail on this topic:
-------------------------------------------------------------------------------------
(Incidentally, this is the third time I've reviewed code today that
looks exactly like this. The other cases are in java.io.FilePermission
and java.util.PropertyPermission. They each have the /*FALLTHROUGH*/
into a set of cases that do nothing but break; and they have similar
("ackbarfaccept") comments. It would be nice if these chunks of code
could be unified, but they differ in a number of fiddly details.)
(The string "ackbarfaccept" occurs in the following files:
1. java/io/FilePermission.java
2. java/net/SocketPermission.java
3. java/util/PropertyPermission.java
4. javax/management/MBeanPermission.java
5. javax/security/auth/kerberos/ServicePermission.java
Hmmm.)
-------------------------------------------------------------------------------------
Thanks.
Hi Brandon,
Hi Stuart,
i had a look at all those classes and it seems to be the same
algorithm. In an
normal project (not the jdk) i would suggest to completely refactor it
and make
some code de-duplication cleanup. But if i thinks on projects like
jigsaw this
can easily get a real problem. What do you think, should we try to
cleanup them
all. Or should we try to make some de-duplication/code-reuse
refactoring.
-- Sebastian
On 12/1/2011 10:18 PM, Sebastian Sickelmann wrote:
Hi Brandon,
i will try to work out a fix for both and cc the review request to
you.
-- Sebastian
Am 01.12.2011 23:54, schrieb Brandon Passanisi:
Hi Sebastian. I was speaking with Stuart Marks earlier today and he
mentioned that the "fallthrough" code in FilePermission.java also
exists in
java.util.PropertyPermission.java. Maybe the code author had done
some
copy/paste when working on these files. Stuart had said that you
might be
planning on doing some work on this after the warnings cleanup work.
If/when you do this work, maybe you can let me know so that I can
sync the
same changes you apply to FilePermission.java to
PropertyPermission.java?
Or, maybe you can apply the same changes yourself to
PropertyPermission.java?
