Webrev updated at

   http://cr.openjdk.java.net/~weijun/8164705/webrev.01

Most suggestions from Alan accepted, including:

1. Using SharedSecrets.getJavaIOFilePermissionAccess() style instead of public functions.

2. jdk.io.permissionsUseCanonicalPath=<boolean>.

3. samepath.sh rewritten in ReadFileOnPath.java.

Still using "ugly" method names. Thinking they are clear enough.

Thanks
Max

On 8/29/2016 20:18, Alan Bateman wrote:
On 25/08/2016 04:55, Weijun Wang wrote:

Hi All

Please take a look at

   http://cr.openjdk.java.net/~weijun/8164705/webrev.00

From the beginning of JDK, FilePermission canonicalizes the input path
and use the result in implies() and equals(). This has been a big
performance hurt and leads to confusing results when symlinks are
involved.

The code change above removes the canonicalization.

This means FilePermission on "/the/current/working/directory/x" no
longer implies that on "x". Since this might bring quite some
compatibility risk, the code change includes some tweaks in permission
checking to make sure an app is still able to read "x" when the
FilePermission granted is on "/the/current/working/directory/x".
However, we still hope the policy to be updated to be consistent of
how a file is actually accessed.

No tweak is devoted to make granting "/this/is/a/symlink" to imply
reading of "/the/actual/target/file", because we think it should not.

This is quite a big behavior change. If it breaks your app/lib, or
does not work with your customized security manager or policy
implementation, please let us know.

Feel free to provide any feedback.

Finally, a new system property "jdk.filepermission.canonicalize" is
introduced and it can be "true", "false", or "compat". The out-of-box
default is "compat", which means no canonicalization with check tweaks.
I've taken a first pass over this. A general comment then it's great to
see FilePermission changed to not do canonicalization. The "new
behavior" approach to match paths and support absolute => relative and
relative => absolute all makes sense.

I agree that having a property to revert to legacy behavior make sense.
Is there any way to reduce this down to just current and legacy
behavior, it's otherwise too complicated to describe the subtle
differences between "false" and "compat". If it can be reduced to two
settings then a name such as
jdk.io.permissionsUseCanonicalPath=<boolean> could be better than the
prototype name.

For the implementation then I think it will cleanup before this is ready
to push. FilePermCompat is a ugly, particularly FilePermission changing
its public fields. Also method names like impliesWithAltFP,
newPermPlusAltPath, ... could be re-visited to make them much clearer.

For the test then it would be good if samepath.sh could be replaced with
a non-shell test.

-Alan

Reply via email to