On Sep 26, 2013, at 4:01 PM, Mandy Chung <mandy.ch...@oracle.com> wrote:
> Hi Henry, > > On 9/25/2013 12:37 AM, Henry Jen wrote: >> Hi, >> >> Please review the update webrev at >> http://cr.openjdk.java.net/~henryjen/ccc/8023524/2/webrev/ > > The doPrivileged block looks okay. It'd be good to limit privileges > by calling the doPrivileged method with a specific set of Permissions. > In this case, it would be FilePermission("<<ALL FILES>>", "write"). > Good point, I will do that. > If "jdk.internal.lambda.dumpProxyClasses" property is set with empty > value, would it be better to default to CWD? ProxyClassesDumper currently > fails with non-existent directory. > That sounds reasonable, I will replace empty string with ".". Personally I prefer explicit value being specified, I always specify "." if I wanted current directory. > ProxyClassesDumper - you may want to use java.nio.file.Path and > Files. I also suggest to follow the convention of classfile pathname > and generate it in the directories reflecting the package name i.e. > DIR/com/example/TestLambda$$Lambda$1.class > > instead of DIR/com.example.TestLambda$$Lambda$1.class, > Make sense to me if that's safe enough to do so. I don't have a strong opinion on this. By avoiding create directories, less chance for malicious class to mess with file system. > For escaping sequence, as Peter pointed out, I presume you can > consider using ParseUtil.encodePath but I am not sure how importance > it has to encode the path as this is for debugging purpose. > No strong opinion in this one. If I have known this, I could have used that. :) In this latest webrev, a simple encoding is implemented which allows unicode characters to pass through, which is probably good enough? Cheers, Henry
