Re: [RFR] 8205525 : Improve exception messages during manifest parsing of jar archives

[Wang Weijun]

Thinking about this again. Looks like the absolute path is not necessary. Even 
if there are multiple files using the same name, they will be in different 
directories, no matter absolute or relative. Suppose the jarPath info is used 
for debugging purpose mostly like the developer can find out what the current 
working directory is and can find the files. *Matthias*: Is the absolute path 
really necessary? Are you working on actual case?

As for the possible global effect of a security property, maybe we can emphasis 
that this is both a security property and system property, and if it’s just for 
one time use, it’s better to use a system property. 

BTW, does the existing value “hostInfo” of the property have a similar problem?

Thanks
Max

>> 在 2018年9月8日，21:42，Sean Mullan  写道：
>> 
>> On 9/7/18 7:58 PM, Weijun Wang wrote:
>> In my understanding, the author deliberately wants to show the absolute 
>> paths when there are multiple jar files with the same name (Ex: a jar hell).
> 
> If you are very familiar with a particular application and understand the 
> risks associated with running it, then I agree that is ok. But security 
> properties apply to all applications using that JDK, and so I don't always 
> think it is practical for an admin to understand every type of application 
> that may be using that JDK and whether or not enabling this property presents 
> a risk.
> 
>> Maybe we can add some more detail in the java.security so an admin knows 
>> what exact impact it has.
> 
> It would be a slippery slope in my opinion. You would have to say something 
> like: "enabling this property may allow untrusted code running under a 
> SecurityManager to access sensitive information such as the user.home system 
> property even if it has not been granted permission to do so."
> 
> I think we should consider not allowing this property to take effect if a 
> SecurityManager is running.
> 
> --Sean
> 
>> --Max
>>> On Sep 8, 2018, at 3:41 AM, Sean Mullan  wrote:
>>> 
>>> On 8/29/18 10:01 AM, Baesken, Matthias wrote:
 Hi Max, thanks for your input .
 I created another webrev , this contains now   the suggested  
 SecurityProperties   class :
 http://cr.openjdk.java.net/~mbaesken/webrevs/8205525.6/
>>> 
>>> java/util/jar/Attributes.java
>>> 
>>> 469 return AccessController.doPrivileged(new 
>>> PrivilegedAction() {
>>> 470 public String run() {
>>> 471 return file.getAbsolutePath() + ":" + lineNumber;
>>> 472 }
>>> 473 });
>>> 
>>> I have a serious concern with the code above.
>>> 
>>> With this change, untrusted code running under a SecurityManager could 
>>> potentially create a JarFile on a non-absolute path ex: "foo.jar", and 
>>> (somehow) cause an IOException to be thrown which would then reveal the 
>>> absolute path of where the jar was located, and thus could reveal sensitive 
>>> details about the system (ex: the user's home directory). It could still be 
>>> hard to exploit, since it would have to be a known jar that exists, and you 
>>> would then need to cause an IOException to be thrown, but it's still 
>>> theoretically possible.
>>> 
>>> In short, this is a more general issue in that it may allow untrusted code 
>>> to access something it couldn't have previously. new 
>>> JarFile("foo.jar").getName() returns "foo.jar", and not the absolute path.
>>> 
>>> Granted this can only be done if the security property is enabled, but I 
>>> believe this is not something administrators should have to know about 
>>> their environment and account for when enabling this property.
>>> 
>>> The above code should be changed to return only what the caller provides to 
>>> JarFile, which is the name of the file (without the full path).
>>> 
>>> --Sean

Re: RFR JDK-8066709 Make some JDK system properties read only

[Wang Weijun]

Not a native English speaker, so my feeling might be incorrect.

Will someone interpret this as that System.getProperty() will return a cached 
value?

I would say “Although getProperty() always returns the last value set by 
setProperty() (I assume this is the current behavior), it is not uncommon that 
consumers of a system property may read it once and cache the value it for 
later use, which means setting the property may not have the desired 
effect”.

I also don’t think it’s worth listing the 4 property names in the spec. Quite 
some other system properties are also cached. Listing them there could further 
suggests that calling getProperty() on them will return the cached value.

Thanks
Max

> 在 2018年6月4日，下午9:32，Roger Riggs  写道：
> 
> Please review a change to make the values of java.home, user.home, user.dir, 
> and user.name
> effectively read-only for internal use.  The values are cached during 
> initialization and the
> cached values are used.
> 
> Webrev:
>   http://cr.openjdk.java.net/~rriggs/webrev-static-property-8066709/
> 
> Issue:
>   https://bugs.openjdk.java.net/browse/JDK-8066709
> 
> CSR:
>   https://bugs.openjdk.java.net/browse/JDK-8204235
> 
> Thanks, Roger
> 
> 

On 8175846: Provide javadoc descriptions for jdk.policytool and jdk.crypto.ec modules

[Wang Weijun]

Hi All

I'm looking at this bug and have several questions:

1. jdk.jartool is newly introduced in jdk9 (well, all modules are new) 
but the main class inside it -- sun.security.tools.policytool.PolicyTool 
-- is already deprecated. Can I also add @deprecated/@Deprecated to the 
module-info.java of this module?


2. I try to run javadoc on this single module but see the following 
error. Not sure what it means. Shall I provide more arguments?


 $ javadoc --module jdk.jartool -d /tmp
javadoc: error - fatal error encountered: 
java.lang.IllegalArgumentException: location is not an output location 
or a module-oriented location: CLASS_PATH
javadoc: error - Please file a bug against the javadoc tool via the Java 
bug reporting page
(http://bugreport.java.com) after checking the Bug Database 
(http://bugs.java.com)
for duplicates. Include error messages and the following diagnostic in 
your report. Thank you.
java.lang.IllegalArgumentException: location is not an output location 
or a module-oriented location: CLASS_PATH
at 
jdk.compiler/com.sun.tools.javac.file.JavacFileManager.checkModuleOrientedOrOutputLocation(JavacFileManager.java:1114)
at 
jdk.compiler/com.sun.tools.javac.file.JavacFileManager.getLocationForModule(JavacFileManager.java:955)
at 
jdk.javadoc/jdk.javadoc.internal.tool.ElementsTable.getModuleLocation(ElementsTable.java:796)
at 
jdk.javadoc/jdk.javadoc.internal.tool.ElementsTable.scanSpecifiedItems(ElementsTable.java:352)
at 
jdk.javadoc/jdk.javadoc.internal.tool.JavadocTool.getEnvironment(JavadocTool.java:189)
at 
jdk.javadoc/jdk.javadoc.internal.tool.Start.parseAndExecute(Start.java:591)
at 
jdk.javadoc/jdk.javadoc.internal.tool.Start.begin(Start.java:424)
at 
jdk.javadoc/jdk.javadoc.internal.tool.Start.begin(Start.java:341)

at jdk.javadoc/jdk.javadoc.internal.tool.Main.execute(Main.java:63)
at jdk.javadoc/jdk.javadoc.internal.tool.Main.main(Main.java:52)

Thanks
Max

Re: RFR 8170900: Issue with FilePermission::implies for wildcard flag(-)

[Wang Weijun]

Ping again.

> On Dec 22, 2016, at 8:23 AM, Wang Weijun  wrote:
> 
> Hi Roger
> 
>> On Dec 20, 2016, at 11:49 PM, Roger Riggs  wrote:
>> 
>> Hi Max,
>> 
>> Comments:
>> 
>> - Is there a better term/phrase to use other than "foo"; it does not appear 
>> elsewhere in the @implNote.
> 
> It appears in the spec of this method:
> 
> *  p's pathname is implied by this object's
> *  pathname. For example, "/tmp/*" implies "/tmp/foo", since
> *  "/tmp/*" encompasses all files in the "/tmp" directory,
> *  including the one named "foo".
> 
>>  The use of "cpath" and "npath" implies that someone is reading the source 
>> code.
> 
> Not really. They also appears in the @implNote of the spec of 
> FilePermission::(String,String):
> 
> * If the value of the system property is set to {@code true}, {@code path}
> * is canonicalized and stored as a String object named {@code cpath}.
> * This means a relative path is converted to an absolute path, a Windows
> * DOS-style 8.3 path is expanded to a long path, and a symbolic link is
> * resolved to its target, etc.
> * 
> * If the value of the system property is set to {@code false}, {@code path}
> * is converted to a {@link java.nio.file.Path} object named {@code npath}
> * after {@link Path#normalize() normalization}. No canonicalization is
> * performed which means the underlying file system is not accessed.
> * If an {@link InvalidPathException} is thrown during the conversion,
> * this {@code FilePermission} will be labeled as invalid.
> 
> I think using the same name in all @implNote is more precise.
> 
>>  The description of the behavior of the implementation should use the same 
>> terminology as the spec.
>> 
>> - The use of "Note" weakens the text as specification language.  It can be 
>> omitted.
> 
> OK.
> 
> I'll use take Xuelei's advice to expand this line to
> 
>  This means "/-" implies "/foo" but not "foo".
> 
>> 
>> - To make the source version more readable, I would keep each statement on 
>> its own line.
> 
> OK.
> 
> Thanks
> Max
> 
>> 
>>   Note that this means "/-" does not imply "foo".
>>   An invalid {@code FilePermission} does not imply any object except for 
>> itself.
>> 
>> Thanks, Roger
>> 
>> On 12/20/2016 2:25 AM, Wang Weijun wrote:
>>> Ping again.
>>> 
>>>> On Dec 14, 2016, at 1:53 PM, Wang Weijun  wrote:
>>>> 
>>>> An clarification is added to FilePermission::implies:
>>>> 
>>>> * @implNote
>>>>   
>>>> * a simple {@code npath} is recursively inside a wildcard {@code npath}
>>>> * if and only if {@code simple_npath.relativize(wildcard_npath)}
>>>> - * is a series of one or more "..". An invalid {@code FilePermission} 
>>>> does
>>>> + * is a series of one or more "..". Note that this means "/-" does not
>>>> + * imply "foo". An invalid {@code FilePermission} does
>>>> * not imply any object except for itself.
>>>> 
>>>> The newly added sentence is
>>>> 
>>>> Note that this means "/-" does not imply "foo".
>>>> 
>>>> JCK has agreed to update their test.
>>>> 
>>>> Since this is just a clarification inside an @implNote and no spec is 
>>>> updated, I suppose no CCC is needed. Please confirm.
>>>> 
>>>> Thanks
>>>> Max
>>>> 
>> 
> 

Re: RFR 8170900: Issue with FilePermission::implies for wildcard flag(-)

[Wang Weijun]

Hi Roger

> On Dec 20, 2016, at 11:49 PM, Roger Riggs  wrote:
> 
> Hi Max,
> 
> Comments:
> 
> - Is there a better term/phrase to use other than "foo"; it does not appear 
> elsewhere in the @implNote.

It appears in the spec of this method:

*  p's pathname is implied by this object's
*  pathname. For example, "/tmp/*" implies "/tmp/foo", since
*  "/tmp/*" encompasses all files in the "/tmp" directory,
*  including the one named "foo".

>   The use of "cpath" and "npath" implies that someone is reading the source 
> code.

Not really. They also appears in the @implNote of the spec of 
FilePermission::(String,String):

* If the value of the system property is set to {@code true}, {@code path}
* is canonicalized and stored as a String object named {@code cpath}.
* This means a relative path is converted to an absolute path, a Windows
* DOS-style 8.3 path is expanded to a long path, and a symbolic link is
* resolved to its target, etc.
* 
* If the value of the system property is set to {@code false}, {@code path}
* is converted to a {@link java.nio.file.Path} object named {@code npath}
* after {@link Path#normalize() normalization}. No canonicalization is
* performed which means the underlying file system is not accessed.
* If an {@link InvalidPathException} is thrown during the conversion,
* this {@code FilePermission} will be labeled as invalid.

I think using the same name in all @implNote is more precise.

>   The description of the behavior of the implementation should use the same 
> terminology as the spec.
> 
> - The use of "Note" weakens the text as specification language.  It can be 
> omitted.

OK.

I'll use take Xuelei's advice to expand this line to

  This means "/-" implies "/foo" but not "foo".

> 
> - To make the source version more readable, I would keep each statement on 
> its own line.

OK.

Thanks
Max

> 
>Note that this means "/-" does not imply "foo".
>An invalid {@code FilePermission} does not imply any object except for 
> itself.
> 
> Thanks, Roger
> 
> On 12/20/2016 2:25 AM, Wang Weijun wrote:
>> Ping again.
>> 
>>> On Dec 14, 2016, at 1:53 PM, Wang Weijun  wrote:
>>> 
>>> An clarification is added to FilePermission::implies:
>>> 
>>>  * @implNote
>>>
>>>  * a simple {@code npath} is recursively inside a wildcard {@code npath}
>>>  * if and only if {@code simple_npath.relativize(wildcard_npath)}
>>> - * is a series of one or more "..". An invalid {@code FilePermission} 
>>> does
>>> + * is a series of one or more "..". Note that this means "/-" does not
>>> + * imply "foo". An invalid {@code FilePermission} does
>>>  * not imply any object except for itself.
>>> 
>>> The newly added sentence is
>>> 
>>>  Note that this means "/-" does not imply "foo".
>>> 
>>> JCK has agreed to update their test.
>>> 
>>> Since this is just a clarification inside an @implNote and no spec is 
>>> updated, I suppose no CCC is needed. Please confirm.
>>> 
>>> Thanks
>>> Max
>>> 
> 

Re: RFR 8170900: Issue with FilePermission::implies for wildcard flag(-)

[Wang Weijun]

> On Dec 22, 2016, at 8:12 AM, Xuelei Fan  wrote:
> 
> I think the note is an example, may not need an additional CCC.

That's always my understanding.

> 
> For easier reading, I may use a contrast example.  For example, "Note that 
> this means "/-" implies "/foo" but not "foo".".

Good advice.

Thanks
Max

> 
> Use the one you like, I'm OK with the either.
> 
> Xuelei
> 
> On 12/21/2016 3:58 PM, Wang Weijun wrote:
>> 
>>> On Dec 22, 2016, at 4:39 AM, Xuelei Fan  wrote:
>>> 
>>> I'm trying to understand this update.  Does "/-" imply "/foo"?
>> 
>> Yes.
>> 
>>> 
>>> Does the following spec can be used to explain the new added note?
>>> 
>>>* if the wildcard flag is "-", the simple pathname's path
>>>* must be recursively inside the wildcard pathname's path.
>> 
>> Yes.
>> 
>> But the precise meaning of "recursively inside" is different between the 
>> pre-jdk9 and jdk9 behaviors. The @implNote explains more.
>> 
>> --Max
>> 
>>> 
>>> Xuelei
>>> 
>>> On 12/19/2016 11:25 PM, Wang Weijun wrote:
>>>> Ping again.
>>>> 
>>>>> On Dec 14, 2016, at 1:53 PM, Wang Weijun  wrote:
>>>>> 
>>>>> An clarification is added to FilePermission::implies:
>>>>> 
>>>>>* @implNote
>>>>>  
>>>>>* a simple {@code npath} is recursively inside a wildcard {@code npath}
>>>>>* if and only if {@code simple_npath.relativize(wildcard_npath)}
>>>>> - * is a series of one or more "..". An invalid {@code 
>>>>> FilePermission} does
>>>>> + * is a series of one or more "..". Note that this means "/-" does 
>>>>> not
>>>>> + * imply "foo". An invalid {@code FilePermission} does
>>>>>* not imply any object except for itself.
>>>>> 
>>>>> The newly added sentence is
>>>>> 
>>>>> Note that this means "/-" does not imply "foo".
>>>>> 
>>>>> JCK has agreed to update their test.
>>>>> 
>>>>> Since this is just a clarification inside an @implNote and no spec is 
>>>>> updated, I suppose no CCC is needed. Please confirm.
>>>>> 
>>>>> Thanks
>>>>> Max
>>>>> 
>>>> 
>> 

Re: RFR 8170900: Issue with FilePermission::implies for wildcard flag(-)

[Wang Weijun]

> On Dec 22, 2016, at 4:39 AM, Xuelei Fan  wrote:
> 
> I'm trying to understand this update.  Does "/-" imply "/foo"?

Yes.

> 
> Does the following spec can be used to explain the new added note?
> 
> * if the wildcard flag is "-", the simple pathname's path
> * must be recursively inside the wildcard pathname's path.

Yes.

But the precise meaning of "recursively inside" is different between the 
pre-jdk9 and jdk9 behaviors. The @implNote explains more.

--Max

> 
> Xuelei
> 
> On 12/19/2016 11:25 PM, Wang Weijun wrote:
>> Ping again.
>> 
>>> On Dec 14, 2016, at 1:53 PM, Wang Weijun  wrote:
>>> 
>>> An clarification is added to FilePermission::implies:
>>> 
>>> * @implNote
>>>   
>>> * a simple {@code npath} is recursively inside a wildcard {@code npath}
>>> * if and only if {@code simple_npath.relativize(wildcard_npath)}
>>> - * is a series of one or more "..". An invalid {@code FilePermission} 
>>> does
>>> + * is a series of one or more "..". Note that this means "/-" does not
>>> + * imply "foo". An invalid {@code FilePermission} does
>>> * not imply any object except for itself.
>>> 
>>> The newly added sentence is
>>> 
>>> Note that this means "/-" does not imply "foo".
>>> 
>>> JCK has agreed to update their test.
>>> 
>>> Since this is just a clarification inside an @implNote and no spec is 
>>> updated, I suppose no CCC is needed. Please confirm.
>>> 
>>> Thanks
>>> Max
>>> 
>> 

Re: JDK 9 RFR of JDK-8156595: java/io/pathNames/GeneralWin32.java fail intermittently on windows-x64

[Wang Weijun]

> For the failing case, the first time it calls checkNames, the "ans" (the 3rd 
> arg) is "current working dir" (/path/scratch/1).

Is it possible to use ./tmp as "ans"?

--Max

Re: RFR 8170900: Issue with FilePermission::implies for wildcard flag(-)

[Wang Weijun]

Ping again.

> On Dec 14, 2016, at 1:53 PM, Wang Weijun  wrote:
> 
> An clarification is added to FilePermission::implies:
> 
>  * @implNote
>
>  * a simple {@code npath} is recursively inside a wildcard {@code npath}
>  * if and only if {@code simple_npath.relativize(wildcard_npath)}
> - * is a series of one or more "..". An invalid {@code FilePermission} 
> does
> + * is a series of one or more "..". Note that this means "/-" does not
> + * imply "foo". An invalid {@code FilePermission} does
>  * not imply any object except for itself.
> 
> The newly added sentence is
> 
>  Note that this means "/-" does not imply "foo".
> 
> JCK has agreed to update their test.
> 
> Since this is just a clarification inside an @implNote and no spec is 
> updated, I suppose no CCC is needed. Please confirm.
> 
> Thanks
> Max
> 

RFR 8171340: HttpNegotiateServer/java test should not use system proxy on Mac

[Wang Weijun]

Please take a review at

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

All "openConnection()" modified to "openConnection(Proxy.NO_PROXY)".

Everything else is whitespace change.

Thanks
Max

RFR 8170900: Issue with FilePermission::implies for wildcard flag(-)

[Wang Weijun]

An clarification is added to FilePermission::implies:

  * @implNote

  * a simple {@code npath} is recursively inside a wildcard {@code npath}
  * if and only if {@code simple_npath.relativize(wildcard_npath)}
- * is a series of one or more "..". An invalid {@code FilePermission} does
+ * is a series of one or more "..". Note that this means "/-" does not
+ * imply "foo". An invalid {@code FilePermission} does
  * not imply any object except for itself.

The newly added sentence is

  Note that this means "/-" does not imply "foo".

JCK has agreed to update their test.

Since this is just a clarification inside an @implNote and no spec is updated, 
I suppose no CCC is needed. Please confirm.

Thanks
Max

Re: RFR 8168979: @implNote for invalid FilePermission

[Wang Weijun]

> On Dec 14, 2016, at 10:11 AM, Xuelei Fan  wrote:
> 
> On 12/13/2016 5:45 PM, Wang Weijun wrote:
>> A major behavior change is that <> now implies an invalid 
>> permission, I hope this is good to minimize incompatibility.
> Looks like two sides of the same coin.  If there is an invalid <> 
> (not existing in practice, I think),

Not existing in theory either. As the change says, invalid happens when the 
conversion of string to NIO Path fails. For <>, there will be no 
such conversion.

> it now implies all;  if no change on this point, <> does not imply 
> invalid one.  The existing behavior looks more safe to me.  What's you 
> concern to make this behavior change?

Just safer. Sometimes an app can recover from a FileNotExistException but not a 
SecurityException.

Thanks
Max

> 
> Otherwise, looks fine to me.
> 
> Xuelei

Re: RFR 8168979: @implNote for invalid FilePermission

[Wang Weijun]

Webrev updated at

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

One comment is moved to its correct position and a typo fixed.

Thanks Daniel for the comment. Can someone with a reviewer hat take a look?

Thanks
Max

> On Dec 12, 2016, at 6:03 PM, Daniel Fuchs  wrote:
> 
> Hi Max,
> 
> Don't count me as reviewer - but I see a mismatched comment
> in the file:
> 
> 209 /**
> 210  * Creates FilePermission objects with special internals.
> 211  * See {@link FilePermCompat#newPermPlusAltPath(Permission)} and
> 212  * {@link FilePermCompat#newPermUsingAltPath(Permission)}.
> 213  */
> 214
> 215 private static final Path here = builtInFS.getPath(
> 216 GetPropertyAction.privilegedGetProperty("user.dir"));
> 
> I guess the comment is a left over from some merge or previous fix?
> 
> 
> Also I noticed the following later on:
> 
> 541  * invalid {@code FilePermission}. Even if two {@code FilePermission}
> 542  * are created with the same invalid path, one does imply the other.
> 
> should this be:
> 
>Even if two {@code FilePermission} are created with the same
>invalid path, one does *not* imply the other.
> 
> best regards,
> 
> -- daniel
> 
> On 12/12/16 09:01, Wang Weijun wrote:
>> Please take a review at
>> 
>>   http://cr.openjdk.java.net/~weijun/8168979/webrev.00/
>> 
>> This further clarifies what an invalid FilePermission behaves. A major 
>> behavior change is that <> now implies an invalid permission, I 
>> hope this is good to minimize incompatibility.
>> 
>> Thanks
>> Max
>> 
> 

Re: RFR 8168979: @implNote for invalid FilePermission

[Wang Weijun]

> On Dec 12, 2016, at 6:03 PM, Daniel Fuchs  wrote:
> 
> Hi Max,
> 
> Don't count me as reviewer - but I see a mismatched comment
> in the file:
> 
> 209 /**
> 210  * Creates FilePermission objects with special internals.
> 211  * See {@link FilePermCompat#newPermPlusAltPath(Permission)} and
> 212  * {@link FilePermCompat#newPermUsingAltPath(Permission)}.
> 213  */
> 214
> 215 private static final Path here = builtInFS.getPath(
> 216 GetPropertyAction.privilegedGetProperty("user.dir"));
> 
> I guess the comment is a left over from some merge or previous fix?

These are 2 different methods: "Plus" and "Using".

> 
> 
> Also I noticed the following later on:
> 
> 541  * invalid {@code FilePermission}. Even if two {@code FilePermission}
> 542  * are created with the same invalid path, one does imply the other.
> 
> should this be:
> 
>Even if two {@code FilePermission} are created with the same
>invalid path, one does *not* imply the other.

Ah, yes.

Thanks
Max

> 
> best regards,
> 
> -- daniel
> 
> On 12/12/16 09:01, Wang Weijun wrote:
>> Please take a review at
>> 
>>   http://cr.openjdk.java.net/~weijun/8168979/webrev.00/
>> 
>> This further clarifies what an invalid FilePermission behaves. A major 
>> behavior change is that <> now implies an invalid permission, I 
>> hope this is good to minimize incompatibility.
>> 
>> Thanks
>> Max
>> 
> 

RFR 8168979: @implNote for invalid FilePermission

[Wang Weijun]

Please take a review at

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

This further clarifies what an invalid FilePermission behaves. A major behavior 
change is that <> now implies an invalid permission, I hope this is 
good to minimize incompatibility.

Thanks
Max

Re: RFR 8170408: LogGeneratedClassesTest.java fails with recent changes

[Wang Weijun]

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

jdk_lang passes on all JPRT platforms.

Thanks
Max

> On Nov 29, 2016, at 9:26 PM, Daniel Fuchs  wrote:
> 
> Hi Max,
> 
> On 29/11/16 12:30, Wang Weijun wrote:
>> Maybe I should use
>> 
>> (p, a) -> p.startsWith(Paths.get("dump/com/example"))
>>&& a.isRegularFile()).count(),
> 
> Yes - something like that.
> But that was just a suggestion.
> 
>> 
>> On Windows it's \.
> 
> I remember I had some trouble getting the filter right.
> 
> best regards,
> 
> -- daniel
> 
>> 
>> I'll run some test now.
>> 
>> Thanks
>> Max
>> 
>>> On Nov 29, 2016, at 8:25 PM, Wang Weijun  wrote:
>>> 
>>> Like this?
>>> 
>>>  (p, a) -> p.toString().startsWith("dump/com/example")
>>> && a.isRegularFile()).count(),
>>> 
>>> Thanks
>>> Max
>>> 
>>>> On Nov 29, 2016, at 7:22 PM, Daniel Fuchs  wrote:
>>>> 
>>>> Hi Max,
>>>> 
>>>> On 29/11/16 06:46, Wang Weijun wrote:
>>>>> http://cr.openjdk.java.net/~weijun/8170408/webrev.00/
>>>>> 
>>>>> A lambda inside JDK is dumped, we should not count it in this test.
>>>> 
>>>> Looks good to me. An alternative might be to only look
>>>> at the files under dump/com/example (I remember modifying
>>>> LogGeneratedClassesTest::testLoggingException for exactly
>>>> that same kind of reasons).
>>>> 
>>>> best regards,
>>>> 
>>>> -- daniel
>>>> 
>>>>> 
>>>>> Thanks
>>>>> Max
>>>>> 
>>>> 
>>> 
>> 
> 

Re: RFR 8170408: LogGeneratedClassesTest.java fails with recent changes

[Wang Weijun]

Maybe I should use

(p, a) -> p.startsWith(Paths.get("dump/com/example"))
&& a.isRegularFile()).count(),

On Windows it's \.

I'll run some test now.

Thanks
Max

> On Nov 29, 2016, at 8:25 PM, Wang Weijun  wrote:
> 
> Like this?
> 
>   (p, a) -> p.toString().startsWith("dump/com/example")
>  && a.isRegularFile()).count(),
> 
> Thanks
> Max
> 
>> On Nov 29, 2016, at 7:22 PM, Daniel Fuchs  wrote:
>> 
>> Hi Max,
>> 
>> On 29/11/16 06:46, Wang Weijun wrote:
>>> http://cr.openjdk.java.net/~weijun/8170408/webrev.00/
>>> 
>>> A lambda inside JDK is dumped, we should not count it in this test.
>> 
>> Looks good to me. An alternative might be to only look
>> at the files under dump/com/example (I remember modifying
>> LogGeneratedClassesTest::testLoggingException for exactly
>> that same kind of reasons).
>> 
>> best regards,
>> 
>> -- daniel
>> 
>>> 
>>> Thanks
>>> Max
>>> 
>> 
> 

Re: RFR 8170408: LogGeneratedClassesTest.java fails with recent changes

[Wang Weijun]

Like this?

   (p, a) -> p.toString().startsWith("dump/com/example")
  && a.isRegularFile()).count(),

Thanks
Max

> On Nov 29, 2016, at 7:22 PM, Daniel Fuchs  wrote:
> 
> Hi Max,
> 
> On 29/11/16 06:46, Wang Weijun wrote:
>> http://cr.openjdk.java.net/~weijun/8170408/webrev.00/
>> 
>> A lambda inside JDK is dumped, we should not count it in this test.
> 
> Looks good to me. An alternative might be to only look
> at the files under dump/com/example (I remember modifying
> LogGeneratedClassesTest::testLoggingException for exactly
> that same kind of reasons).
> 
> best regards,
> 
> -- daniel
> 
>> 
>> Thanks
>> Max
>> 
> 

RFR 8170408: LogGeneratedClassesTest.java fails with recent changes

[Wang Weijun]

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

A lambda inside JDK is dumped, we should not count it in this test.

Thanks
Max

Re: RFR 8170364: FilePermission path modified during merge

[Wang Weijun]

Hi Alan

Updated webrev at

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

Changes since webrev.00:

- a private constructor that can clones 4 fields and modifies 5 others

- using lambda

- test enhancement

Thanks
Max

Re: RFR 8170364: FilePermission path modified during merge

[Wang Weijun]

> On Nov 27, 2016, at 7:13 PM, Wang Weijun  wrote:
> 
>> 
>> On Nov 27, 2016, at 6:12 PM, Alan Bateman  wrote:
>> 
>> On 26/11/2016 08:54, Wang Weijun wrote:
>> 
>>> Please take a review at
>>> 
>>>   http://cr.openjdk.java.net/~weijun/8170364/webrev.00/
>>> 
>>> The compatibility layer introduced in the new FilePermission implementation 
>>> requires one FilePermission to imply another with either a relative path or 
>>> an absolute path. This is solved with a private field npath2 inside. When 
>>> this field is set, the permission's name is modified a little (JDK-8168127) 
>>> so that when adding to a FilePermissionCollection, it is not confused with 
>>> one without the field. Unfortunately, this modified name is reused in the 
>>> merge to create a new "merged" FilePermission which contains a malformed 
>>> path now.
>>> 
>>> This fix creates a new non-public method to create this "merged" 
>>> FilePermission.
>>> 
>>> I checked other places and seems the name is not used to create a new 
>>> FilePermission.
>>> 
>> Ideally FilePermission (and all permissions) would have final fields, I hope 
>> that can be fixed some day. 
> 
> Me too, but we have that huge init() method now.
> 
>> In the mean-time then having withNewActions create a new FilePermission and 
>> then mutate it looks a bit ugly, can't you just introduce a new non-public 
>> constructor to create it with the effective mask?
> 
> The private "clone" constructor is now used in 3 places, all with a mutation 
> right after (lines 267, 280, 993). I've taken care that only newly created 
> objects are mutated this way. Creating 3 new constructors looks like too many 
> duplicated codes.

Do you still have a strong opinion?

> 
>> 
>> In passing then maybe the comment at L1063-1065 can be re-examined and it 
>> looks to be stale (the specific bootstrapping issue mentioned was fixed in 
>> 2015).
> 
> I can try.

Changed to lambda. Tests on -testset core most pass. 
java/lang/invoke/lambda/LogGeneratedClassesTest.java still fails but it already 
failed before my change.

> 
>> 
>> The test is one specific scenario and I wonder if you've considered beefing 
>> this up to other scenarios and other targets so that it's more broadly 
>> testing the merging scenarios.
> 
> I can add 2 more lines on checking the absolute path, and maybe with delete, 
> execute and readlink (1+1+1+1, 2+2, 3+1 etc). The merge is only about the 
> name and any name is the same, so it seems unnecessary to try other names.

Test enhanced. Permissions are granted in "read", "write", "delete", "execute", 
or "read,write", "delete,execute", or "read,write,delete", "execute", or 
"read,write,delete,execute", and 2^4-1 combinations of actions for both "x" and 
"/abs/x" checked.

If you are OK with the above, I'll post an updated webrev.

Thanks
Max

> 
> Thanks
> Max
> 
> 
>> 
>> -Alan

Re: RFR 8170364: FilePermission path modified during merge

[Wang Weijun]

> On Nov 27, 2016, at 6:12 PM, Alan Bateman  wrote:
> 
> On 26/11/2016 08:54, Wang Weijun wrote:
> 
>> Please take a review at
>> 
>>http://cr.openjdk.java.net/~weijun/8170364/webrev.00/
>> 
>> The compatibility layer introduced in the new FilePermission implementation 
>> requires one FilePermission to imply another with either a relative path or 
>> an absolute path. This is solved with a private field npath2 inside. When 
>> this field is set, the permission's name is modified a little (JDK-8168127) 
>> so that when adding to a FilePermissionCollection, it is not confused with 
>> one without the field. Unfortunately, this modified name is reused in the 
>> merge to create a new "merged" FilePermission which contains a malformed 
>> path now.
>> 
>> This fix creates a new non-public method to create this "merged" 
>> FilePermission.
>> 
>> I checked other places and seems the name is not used to create a new 
>> FilePermission.
>> 
> Ideally FilePermission (and all permissions) would have final fields, I hope 
> that can be fixed some day. 

Me too, but we have that huge init() method now.

> In the mean-time then having withNewActions create a new FilePermission and 
> then mutate it looks a bit ugly, can't you just introduce a new non-public 
> constructor to create it with the effective mask?

The private "clone" constructor is now used in 3 places, all with a mutation 
right after (lines 267, 280, 993). I've taken care that only newly created 
objects are mutated this way. Creating 3 new constructors looks like too many 
duplicated codes.

> 
> In passing then maybe the comment at L1063-1065 can be re-examined and it 
> looks to be stale (the specific bootstrapping issue mentioned was fixed in 
> 2015).

I can try.

> 
> The test is one specific scenario and I wonder if you've considered beefing 
> this up to other scenarios and other targets so that it's more broadly 
> testing the merging scenarios.

I can add 2 more lines on checking the absolute path, and maybe with delete, 
execute and readlink (1+1+1+1, 2+2, 3+1 etc). The merge is only about the name 
and any name is the same, so it seems unnecessary to try other names.

Thanks
Max


> 
> -Alan
> 
> 

RFR 8170364: FilePermission path modified during merge

[Wang Weijun]

Please take a review at

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

The compatibility layer introduced in the new FilePermission implementation 
requires one FilePermission to imply another with either a relative path or an 
absolute path. This is solved with a private field npath2 inside. When this 
field is set, the permission's name is modified a little (JDK-8168127) so that 
when adding to a FilePermissionCollection, it is not confused with one without 
the field. Unfortunately, this modified name is reused in the merge to create a 
new "merged" FilePermission which contains a malformed path now.

This fix creates a new non-public method to create this "merged" FilePermission.

I checked other places and seems the name is not used to create a new 
FilePermission.

Thanks
Max

Re: RFR 8167646: Better invalid FilePermission

[Wang Weijun]

Thanks Roger. Can you please also take a look on another code review request at 
http://mail.openjdk.java.net/pipermail/core-libs-dev/2016-October/044260.html?

--Max

> On Oct 22, 2016, at 2:18 AM, Roger Riggs  wrote:
> 
> Hi Max,
> 
> Looks ok.
> An additional reviewer would be good too.
> 
> Roger
> 
> 
> On 10/20/2016 4:51 AM, Wang Weijun wrote:
>> Please review the code change at
>> 
>>http://cr.openjdk.java.net/~weijun/8167646/webrev.00/
>> 
>> A new flag invalid is added so invalid FilePermissions (invalid Path) do not 
>> equal or imply or is implied by anything else except for itself.
>> 
>> Thanks
>> Max
>> 
> 

RFR 8167646: Better invalid FilePermission

[Wang Weijun]

Please review the code change at

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

A new flag invalid is added so invalid FilePermissions (invalid Path) do not 
equal or imply or is implied by anything else except for itself.

Thanks
Max

RFR 8168127: FilePermissionCollection merges incorrectly

[Wang Weijun]

Please review the code change at

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

Two changes:

1. npath2 is considered in equals and hashCode of FilePermission, so 2 objects 
with different npath2 can be added to a map and different entries.

2. special name for newPermUsingAltPath and newPermPlusAltPath results, so 
FilePermissionCollection::add will not merge one with the original.

Thanks
Max

Re: RFR (JAXP) JDK-8167478 javax/xml/jaxp/unittest/parsers/Bug6341770.java failed with "java.security.AccessControlException: access denied ("java.io.FilePermission" "sko?ice")"

[Wang Weijun]

http://hg.openjdk.java.net/jdk9/dev/jaxp/rev/037c095ba0c345edbeaaab52fda913a76c3930c0

My understanding is that if the changeset already has your name as the author 
then there is no need to add your name again into Contributed-by.

--Max

> On Oct 17, 2016, at 10:13 AM, Frank Yuan  wrote:
> 
> Alright, thank you! Then I will check it in.
> 
> Frank

Re: RFR 9: JEP 290: Filter Incoming Serialization Data

[Wang Weijun]

The ObjectInputFilter interface has only one method. Are we expecting more 
methods to be added later and maybe some people will only be interested in a 
new method? If yes, I suggest we provide a default implementation of the 
current method to return ALLOWED.

--Max

> On Jul 19, 2016, at 10:02 PM, Roger Riggs  wrote:
> 
> Please review the design, implementation, and tests of JEP 290: Filter 
> Incoming Serialization Data[1]

os.name will be "macOS"? (was Re: RFR 9 : 8160370 : System.getProperty("os.version") returns "Unknown" on Mac)

[Wang Weijun]

I have an off-topic question:

Will os.name be macOS for 10.12? I have several places checking "if 
(!osname.contains("OS X"))", is there a helper method I can check for this in 
the future no matter if it's running on pre- or post-10.12?

Thanks
Max

> On Jul 1, 2016, at 2:23 AM, Brent Christian  
> wrote:
> 
> On 6/30/16 9:53 AM, Brent Christian wrote:
>> 
>> When the minimum Mac build platform is SDK 10.10, we'll be able to call
>> operatingSystemVersion directly without using msgSend.  We can also
>> consider removing this then.
> 
> FYI:
> https://bugs.openjdk.java.net/browse/JDK-8160676
> 
> -Brent

Get user.dir as a symlink not resolved

[Wang Weijun]

I'm on a Mac inside /tmp, which is a symlink to /private/tmp.

System.getProperty("user.dir") shows me "/private/tmp". Is there a way to get 
"/tmp" which is exactly what `pwd` return? I only like "/private/tmp" if I 
called "cd /private/tmp".

Thanks
Max

Re: JDK 9 RFR of JDK-8159330: Improve deprecation text for Class.newInstance

[Wang Weijun]

Why not just clazz.getConstructor().newInstance()?

> + * can be replaced by
> + *
> + * {@code
> + * clazz.getConstructor(new Class[0]).newInstance((Object[])null);
> + * }

Re: RFR: regex changes -- sun.security.util.Debug issue

[Wang Weijun]

Security-dev,

If we can live with "engine=keystore" happily, why not just make the whole 
string lowercase and search for "permission=java.io.filepermission"? I don't 
think there are permission types or URL names that are only different in cases. 
Although file names are case-sensitive in Unix, I doubt if any customer will 
complain if we show him info on /ETC even if what he sets is "codebase=/etc".

We can add new methods like hasCodebase(URL) and hasPermission(Class) to hide the search details inside Debug.

Thanks
Max

> On May 10, 2016, at 1:36 PM, Xueming Shen  wrote:
> 
> Hi,
> 
> While testing for the attached regex changes, a fatal vm init error was 
> triggered for test
> case with -Djava.security.debug=xyz turned on, as showed in following 
> stacktrace.
> 
> It appears sun.security.util.Debug is being initialized even before the 
> lambda is ready
> for use, and unfortunately it uses j.u.regex (for its args parsing), which is 
> being migrated
> to use lambda function in the proposed regex change.
> 
> Since Debug is the only class now triggers j.u.regex -> lambda during 
> initialization, it
> is suggested to update/rewrite the related method in Debug to NOT use 
> j.u.regex to
> solve/workaround this specific initialization issue.
> 
> The webrev below has been updated to include such a change.
> 
> http://cr.openjdk.java.net/~sherman/regexBackTrack.Lamnda.CanonEQ/webrev/ 
> 
> The sun.security.util.Debug related change is at
> 
> http://cr.openjdk.java.net/~sherman/regexBackTrack.Lamnda.CanonEQ/webrev/src/java.base/share/classes/sun/security/util/Debug.java.sdiff.html
> 
> Can security-dev guys help take a look if this is an acceptable/reasonable 
> approach.
> 
> Thanks,
> Sherman
> 
> -
>  Caused by: java.lang.ExceptionInInitializerError
> 
>  
>   at 
> jdk.internal.loader.BootLoader.loadClassOrNull(java.base/BootLoader.java:110)
>   at 
> java.lang.invoke.BoundMethodHandle$Factory$1.apply(java.base/BoundMethodHandle.java:497)
>   at 
> java.lang.invoke.BoundMethodHandle$Factory$1.apply(java.base/BoundMethodHandle.java:492)
> ...
>   at 
> java.lang.invoke.MethodHandleNatives.linkCallSite(java.base/MethodHandleNatives.java:240)
>   at java.util.regex.Pattern.(java.base/Pattern.java:5682)
>   at sun.security.util.Debug.marshal(java.base/Debug.java:247)
>   at sun.security.util.Debug.(java.base/Debug.java:59)
>   at 
> java.security.ProtectionDomain.(java.base/ProtectionDomain.java:142)
>   at 
> jdk.internal.misc.InnocuousThread.(java.base/InnocuousThread.java:129)
>   at 
> jdk.internal.ref.CleanerFactory$1$1.run(java.base/CleanerFactory.java:48)
>   at 
> jdk.internal.ref.CleanerFactory$1$1.run(java.base/CleanerFactory.java:45)
>   at java.security.AccessController.doPrivileged(java.base/Native Method)
>   at 
> jdk.internal.ref.CleanerFactory$1.newThread(java.base/CleanerFactory.java:45)
>   at jdk.internal.ref.CleanerImpl.start(java.base/CleanerImpl.java:116)
>   at java.lang.ref.Cleaner.create(java.base/Cleaner.java:203)
>   at 
> jdk.internal.ref.CleanerFactory.(java.base/CleanerFactory.java:42)
>   at sun.nio.fs.NativeBuffer.(java.base/NativeBuffer.java:60)
>   at 
> sun.nio.fs.NativeBuffers.allocNativeBuffer(java.base/NativeBuffers.java:49)
>   at 
> sun.nio.fs.UnixNativeDispatcher.copyToNativeBuffer(java.base/UnixNativeDispatcher.java:44)
>   at 
> sun.nio.fs.UnixNativeDispatcher.stat(java.base/UnixNativeDispatcher.java:306)
>   at 
> sun.nio.fs.UnixFileSystemProvider.isRegularFile(java.base/UnixFileSystemProvider.java:514)
>   at java.nio.file.Files.isRegularFile(java.base/Files.java:2244)
>   at 
> java.lang.module.ModuleFinder.ofSystem(java.base/ModuleFinder.java:165)
>   at 
> jdk.internal.module.ModuleBootstrap.boot(java.base/ModuleBootstrap.java:105)
>   at java.lang.System.initPhase2(java.base/System.java:1924)
> 
> -
> 
> 
> On 3/18/16 1:05 PM, Xueming Shen wrote:
>> Hi, 
>> 
>> There are couple regex related changes waiting for review. I have pull them 
>> together here (with the notes) to make it easy to review. 
>> 
>> http://cr.openjdk.java.net/~sherman/regexBackTrack.Lamnda.CanonEQ/webrev/ 
>> 
>> (1) Exponential backtracking 
>> 
>> Note: 
>> http://cr.openjdk.java.net/~sherman/regexBackTrack.Lamnda.CanonEQ/backtracking
>> 
>> https://bugs.openjdk.java.net/browse/JDK-6328855 
>> https://bugs.openjdk.java.net/browse/JDK-6192895 
>> https://bugs.openjdk.java.net/browse/JDK-6345469 
>> https://bugs.openjdk.java.net/browse/JDK-6988218 
>> https://bugs.openjdk.java.net/browse/JDK-6693451 
>> https://bugs.openjdk.java.net/browse/JDK-7006761 
>> https://bugs.openjdk.java.net/browse/JDK-8140212 
>> 
>> (2) Anonymous class to lambda function cleanup 
>> 
>> Note: 
>> http://cr.openjdk.java.net/~sherman/regexBackTrack.Lamnd

Re: RFR: 8154231: Simplify access to System properties from JDK code

[Wang Weijun]

This is quite convenient. We not cover the other modules?

exports sun.security.action to
java.desktop,
java.security.jgss,
jdk.crypto.pkcs11;

Thanks
Max

> On Apr 20, 2016, at 10:44 PM, Claes Redestad  
> wrote:
> 
> Hello,
> 
> now that the sun.security.action package is encapsulated we can simplify 
> internal code to get System properties.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8154231
> Webrev: http://cr.openjdk.java.net/~redestad/8154231/webrev.01/
> 
> This adds a few convenience methods to GetPropertyAction[1] and 
> GetIntegerAction[2]. Since the code calling into this can be streamlined, 
> this leads to a net static footprint reduction of the java.base module and a 
> tiny startup improvement.
> 
> Thanks!
> 
> /Claes
> 
> [1] 
> http://cr.openjdk.java.net/~redestad/8154231/webrev.01/src/java.base/share/classes/sun/security/action/GetPropertyAction.java.udiff.html
> [2] 
> http://cr.openjdk.java.net/~redestad/8154231/webrev.01/src/java.base/share/classes/sun/security/action/GetIntegerAction.java.udiff.html

Fwd: How do I import a sun.* class in jshell

[Wang Weijun]

Does anyone here know the answer?

It will be quite useful for me if I am either authoring an internal class or 
using it when implementing a public API.

Thanks
Max

> Begin forwarded message:
> 
> From: Wang Weijun 
> Subject: How do I import a sun.* class in jshell
> Date: April 13, 2016 at 5:22:02 PM GMT+8
> To: kulla-...@openjdk.java.net
> 
> 
> $ jshell -J-XaddExports:java.base/sun.security.provider=ALL-UNNAMED
> |  Welcome to JShell -- Version 9-internal
> |  Type /help for help
> 
> -> import sun.security.provider.SecureRandom
> |  Error:
> |  package sun.security.provider does not exist
> |  import sun.security.provider.SecureRandom;
> | ^^
> 
> Thanks
> Max
> 

Re: Code Review Request, 8152237 Support BigInteger.TWO

[Wang Weijun]

> On Mar 23, 2016, at 7:23 PM, Xuelei Fan  wrote:
> 
> On 3/23/2016 5:44 PM, Wang Weijun wrote:
>> Then why not fix the 2 bugs in a single changeset?
>> 
> Both need spec update approval.  As they are completely different spec
> update, better to update in 2 enhancements.
> 
> As you have concerns here, I removed ObjectIdentifier.java from this
> update.  See the new webrev:
> 
>   http://cr.openjdk.java.net/~xuelei/8152237/webrev.01/

Looks good to me.

--Max

> 
> Xuelei

Re: Code Review Request, 8152237 Support BigInteger.TWO

[Wang Weijun]

Then why not fix the 2 bugs in a single changeset?

--Max

> 在 2016年3月23日，17:06，Xuelei Fan  写道：
> 
>> On 3/23/2016 3:34 PM, Wang Weijun wrote:
>> 
>>> On Mar 23, 2016, at 12:48 PM, Xuelei Fan  wrote:
>>> 
>>> On 3/23/2016 12:10 PM, Wang Weijun wrote:
>>>> Only 3 files touched. Are you going to make the 
>>>> s/BigInteger.valueOf(2)/BigInteger.TWO/ changes in other files with 
>>>> another bug fix?
>>> There are also uses in security components.  I will make the update in 
>>> another bug.
>> 
>> I see. But why is ObjectIdentifier.java included in this fix?
> It happens that the other bug touch those files, but
> ObjectIdentifier.java is not related to that bug.
> 
> Does it make sense?
> 
> Thanks,
> Xuelei
> 
>> In you only keep BigInteger and BigDecimal, then I have no other comment.
>> 
>> Thanks
>> Max
>> 
>>> 
>>> Thanks,
>>> Xuelei
>>> 
>>>> Thanks
>>>> Max
>>>> 
>>>>> On Mar 23, 2016, at 11:26 AM, Xuelei Fan  wrote:
>>>>> 
>>>>> Hi,
>>>>> 
>>>>> Please review the update for the supporting of BigInteger.TWO:
>>>>> 
>>>>>  http://cr.openjdk.java.net/~xuelei/8152237/webrev/
>>>>> 
>>>>> BigInteger.valueOf(2) is a common BigInteger value used in binary and 
>>>>> cryptography operation calculation.  The BigInteger.TWO is not exported, 
>>>>> and hence BigInteger.valueOf(2) is used instead in applications and JDK 
>>>>> components.  The export of static BigInteger.TWO can improve performance 
>>>>> and simplify existing code.
>>>>> 
>>>>> Thanks,
>>>>> Xuelei
> 

Re: Code Review Request, 8152237 Support BigInteger.TWO

[Wang Weijun]

> On Mar 23, 2016, at 12:48 PM, Xuelei Fan  wrote:
> 
> On 3/23/2016 12:10 PM, Wang Weijun wrote:
>> Only 3 files touched. Are you going to make the 
>> s/BigInteger.valueOf(2)/BigInteger.TWO/ changes in other files with another 
>> bug fix?
>> 
> There are also uses in security components.  I will make the update in 
> another bug.

I see. But why is ObjectIdentifier.java included in this fix?

In you only keep BigInteger and BigDecimal, then I have no other comment.

Thanks
Max

> 
> Thanks,
> Xuelei
> 
>> Thanks
>> Max
>> 
>>> On Mar 23, 2016, at 11:26 AM, Xuelei Fan  wrote:
>>> 
>>> Hi,
>>> 
>>> Please review the update for the supporting of BigInteger.TWO:
>>> 
>>>   http://cr.openjdk.java.net/~xuelei/8152237/webrev/
>>> 
>>> BigInteger.valueOf(2) is a common BigInteger value used in binary and 
>>> cryptography operation calculation.  The BigInteger.TWO is not exported, 
>>> and hence BigInteger.valueOf(2) is used instead in applications and JDK 
>>> components.  The export of static BigInteger.TWO can improve performance 
>>> and simplify existing code.
>>> 
>>> Thanks,
>>> Xuelei
>> 
> 

Re: Code Review Request, 8152237 Support BigInteger.TWO

[Wang Weijun]

Only 3 files touched. Are you going to make the 
s/BigInteger.valueOf(2)/BigInteger.TWO/ changes in other files with another bug 
fix?

Thanks
Max

> On Mar 23, 2016, at 11:26 AM, Xuelei Fan  wrote:
> 
> Hi,
> 
> Please review the update for the supporting of BigInteger.TWO:
> 
>   http://cr.openjdk.java.net/~xuelei/8152237/webrev/
> 
> BigInteger.valueOf(2) is a common BigInteger value used in binary and 
> cryptography operation calculation.  The BigInteger.TWO is not exported, and 
> hence BigInteger.valueOf(2) is used instead in applications and JDK 
> components.  The export of static BigInteger.TWO can improve performance and 
> simplify existing code.
> 
> Thanks,
> Xuelei

Re: RFR: 8147607: Remove test library dependency on sun.security.tools.jarsigner.Main

[Wang Weijun]

Shouldn't you also include the FileOuputStream in try-with-resources?

--Max

> On Jan 28, 2016, at 5:32 PM, Chris Hegarty  wrote:
> 
> 
> On 28 Jan 2016, at 00:35, Steve Drach  wrote:
> 
>> Please review a small change to the zipfs test library.
>> 
>> Issue: https://bugs.openjdk.java.net/browse/JDK-8147607
>> Webrev: http://cr.openjdk.java.net/~sdrach/8147607/webrev
>> 
>> This change uses the public jdk.security.jarsigner.JarSigner API rather than 
>> the API exposed by sun.security.tools.jarsigner.
> 
> Thanks for doing this Steve. Your changes look fine.
> 
> -Chris.

Re: RFR: JDK-8145549 Add support for Visual Studio 2015 Community edition

[Wang Weijun]

> On Jan 14, 2016, at 11:00 PM, Magnus Ihse Bursie 
>  wrote:
> 
> On 2015-12-18 15:11, Wang Weijun wrote:
>> Hi Vinnie
>> 
>> I take a look and it includes a change for 
>> src/jdk.crypto.mscapi/windows/native/libsunmscapi/security.cpp in the 
>> Java_sun_security_mscapi_KeyStore_getKeyLength() function.
>> 
>> It seems there is no sun.security.mscapi.KeyStore#getKeyLength on the java 
>> side. Is the function useless now?
> 
> Max,
> 
> Is your intention here that you think the patch should remove the entire  
> Java_sun_security_mscapi_KeyStore_getKeyLength function?

Yes, I hope so.

--Max

> 
> /Magnus
> 
>> 
>> Thanks
>> Max
>> 
>>> On Dec 16, 2015, at 9:50 PM, Magnus Ihse Bursie 
>>>  wrote:
>>> 
>>> There is an interest from the community to build OpenJDK using Visual 
>>> Studio 2015 Community edition.
>>> 
>>> This patch is provided by Timo Kinnunen . I am 
>>> sponsoring this patch.
>>> 
>>> The changes to the source code files are mostly casts to uintptr_t, but 
>>> there are some other changes as well.
>>> 
>>> I'm not quite sure who's the owner of all these files. If I'm missing some 
>>> group, please help me and forward the mail to them.
>>> 
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8145549
>>> WebRev: 
>>> http://cr.openjdk.java.net/~ihse/JDK-8145549-vs2015-community-edition/webrev.01
>>> 
>>> /Magnus
> 

Re: RFR [9] 8145544: Move sun.misc.VM to jdk.internal.misc

[Wang Weijun]

I am OK with the change for krb5 in both src and test.

Thanks
Max

> On Jan 4, 2016, at 10:02 PM, Chris Hegarty  wrote:
> 
> sun.misc.VM provides a low-level interface for a small number
> of specific operations with the VM. In preparation for JEP 260,
> this class should be moved out of sun.misc and located in a
> non-exported package [.
> 
> http://cr.openjdk.java.net/~chegar/8145544/00/
> 
> Note: as in other areas some tests that require access to
> internal APIs have been updated to use types from a more
> stable package, sun.security.x509.
> 
> -Chris.
> 
> [1] https://bugs.openjdk.java.net/browse/JDK-8145544

Re: RFR: JDK-8145549 Add support for Visual Studio 2015 Community edition

[Wang Weijun]

Hi Vinnie

I take a look and it includes a change for 
src/jdk.crypto.mscapi/windows/native/libsunmscapi/security.cpp in the 
Java_sun_security_mscapi_KeyStore_getKeyLength() function.

It seems there is no sun.security.mscapi.KeyStore#getKeyLength on the java 
side. Is the function useless now?

Thanks
Max

> On Dec 16, 2015, at 9:50 PM, Magnus Ihse Bursie 
>  wrote:
> 
> There is an interest from the community to build OpenJDK using Visual Studio 
> 2015 Community edition. 
> 
> This patch is provided by Timo Kinnunen . I am 
> sponsoring this patch.
> 
> The changes to the source code files are mostly casts to uintptr_t, but there 
> are some other changes as well.
> 
> I'm not quite sure who's the owner of all these files. If I'm missing some 
> group, please help me and forward the mail to them.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8145549
> WebRev: 
> http://cr.openjdk.java.net/~ihse/JDK-8145549-vs2015-community-edition/webrev.01
> 
> /Magnus

Re: RFR [9] 8144480: Remove test dependencies on sun.misc.BASE64Encoder and BASE64Decoder

[Wang Weijun]

> On Dec 2, 2015, at 11:26 PM, Chris Hegarty  wrote:
> 
> Thanks Max,
> 
> I'm ok with this version, if you are. I'll include it in the final push.

Please.

--Max

> 
> -Chris.
> 
> On 02/12/15 15:13, Wang Weijun wrote:
>> 
>>> On Dec 2, 2015, at 10:52 PM, Wang Weijun  wrote:
>>> 
>>> My fault to use an internal class. I should have simply used the hex 
>>> encoding. Please wait a while and I'll send you a fix.
>>> 
>>> Thanks
>>> Max

Re: RFR [9] 8144480: Remove test dependencies on sun.misc.BASE64Encoder and BASE64Decoder

[Wang Weijun]

> On Dec 2, 2015, at 10:52 PM, Wang Weijun  wrote:
> 
> My fault to use an internal class. I should have simply used the hex 
> encoding. Please wait a while and I'll send you a fix.
> 
> Thanks
> Max

Re: RFR [9] 8144480: Remove test dependencies on sun.misc.BASE64Encoder and BASE64Decoder

[Wang Weijun]

My fault to use an internal class. I should have simply used the hex encoding. 
Please wait a while and I'll send you a fix.

Thanks
Max

> On Dec 2, 2015, at 10:15 PM, Chris Hegarty  wrote:
> 
> On 02/12/15 14:03, Alan Bateman wrote:
>> 
>> On 02/12/2015 12:08, Chris Hegarty wrote:
>>> The regression tests in the jdk should be updated, where possible, to
>>> use java.util.Base64.
>>> 
>>> http://cr.openjdk.java.net/~chegar/Base64-test-updates/webrev/
>> Should S11N be updated to serialize with JDK 8 so that the core
>> reflection code isn't needed?
> 
> This is certainly possible and would make the test simpler, but
> I followed the comments in the test ( it needs to be runnable on
> older JDKs ).  I think this is a reasonable requirement since the
> test contains bytes array that were generated on older JDKs. Of
> course, the test could contain the older JDK code commented out.
> If someone really wants to run it with 1.6, then they just make
> the simple edits.
> 
> I'm ok with any of these solutions, or just removing the possibility
> of running on older JDKs.
> 
> -Chris.

Re: RFR 8142927: Feed some text to STDIN in ProcessTools.executeProcess()

[Wang Weijun]

> On 2015年11月13日, at 下午9:44, Roger Riggs  wrote:
> 
> Hi Max,
> 
> It would be cleaner to create and use a PrintStream to send the bytes
> to the process.  We don't want to encourage the poor practice of using 
> getBytes().

Like this?

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

> Please also document that the exit status of the process is ignored.

It is not really ignored. Process remembers it and OutputAnalyzer can retrieve 
it later.

Thanks
Max

> 
> Roger
> 
> 
> On 11/13/15 3:21 AM, Wang Weijun wrote:
>> Hi All
>> 
>> 8142927: Feed some text to STDIN in ProcessTools.executeProcess()
>> http://cr.openjdk.java.net/~weijun/8142927/webrev.00/
>> 
>> With this change, you can call
>> 
>>   ProcessTools.executeProcess(new ProcessBuilder("keytool -printcert", 
>> certInAscii)
>> 
>> which means "echo $certInAscii | keytool -printcert".
>> 
>> Not sure if there is already an existing way to do this easily. Or maybe 
>> someone might think byte[] is better.
>> 
>> Do I need to write a test for testlibrary?
>> 
>> Thanks
>> Max
>> 
>> 
>> 
> 

Re: RFR 8143015/9: 5 tests fail with error "Can't find source for class: java.util.stream.OpTestCase"

[Wang Weijun]

Hi Felix

The fix looks fine.

Do you want me pushing the changeset? If yes, please tell me what the full 
comment should be.

Thanks
Max

> On Nov 16, 2015, at 11:44 AM, Felix Yang  wrote:
> 
> Hi,
>please review the following fix for 5 broken test cases, which was 
> introduced by JDK-8142996. These tests refer with 
> java.util.stream.OpTestCase, which was relocated from 
> test/java/util/stream/bootlib/ to test/java/util/stream/bootlib/java.base/
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8143015
> Webrev: http://cr.openjdk.java.net/~xiaofeya/8143015/webrev.00/
> 
> Thanks,
> Felix

RFR 8142926: OutputAnalyzer's shouldXXX() calls return this

[Wang Weijun]

Hi All

8142926: OutputAnalyzer's shouldXXX() calls return this
http://cr.openjdk.java.net/~weijun/8142926/webrev.00/

With this change, you can write output.shouldA().shouldB().shouldC().

Do I need to write a test for testlibrary?

Thanks
Max

RFR 8142927: Feed some text to STDIN in ProcessTools.executeProcess()

[Wang Weijun]

Hi All

8142927: Feed some text to STDIN in ProcessTools.executeProcess()
http://cr.openjdk.java.net/~weijun/8142927/webrev.00/

With this change, you can call

  ProcessTools.executeProcess(new ProcessBuilder("keytool -printcert", 
certInAscii)

which means "echo $certInAscii | keytool -printcert".

Not sure if there is already an existing way to do this easily. Or maybe 
someone might think byte[] is better.

Do I need to write a test for testlibrary?

Thanks
Max

Re: dlsym(RTLD_DEFAULT, "getentropy") return non-NULL on Mac

[Wang Weijun]

> On Nov 8, 2015, at 7:18 PM, Dmitry Samersoff  
> wrote:
> 
> Wang Weijun,
> 
>> The function is rather new in the latest Solaris beta [1] and it's
>> preferred to reading from /dev/random. There are already people
>> suggest adding it to Linux. If I use simply using dlsym then it will
>> automatically work on all current and future platforms. In fact, I
>> was planning to write
> 
> 1. Please, check libc only not entire process image.

dlopen("libc.so", RTLD_LAZY)?

This works on Solaris but on Linux seems I have to use "libc.so.6".

> 
> 
> 2. OS X random system is a different story: /dev/random never blocks and
> returns the output of Yarrow PRNG.  Also OS X uses SecurityServer
> process to feed entropy pool.
> 
> So IMHO, it's better to don't attempt to use other functions on OS X
> until it appears officially.

OK.

> 
> 3. Please notice that:
> 
>   /dev/random will block if you request more bits than entropy pool can
> provide.
> 
>   but (according to manual page) getentropy will return immediately
> with error if less that requested bytes of entropy is available and you
> can't request more than 256 bytes of entropy at once.
> 
>it might require changes in uplevel logic.

Not much. I won't need much entropy.

Thanks
Max

> 
> -Dmitry

Re: dlsym(RTLD_DEFAULT, "getentropy") return non-NULL on Mac

[Wang Weijun]

> On Nov 8, 2015, at 4:29 AM, Dmitry Samersoff  
> wrote:
> 
> Wang Weijun,
> 
> 1. RTLD_DEFAUL call is expensive and dangerous because it cause symbol
> search across all loaded images. So it can pick up something absolutely
> irrelevant to your expectations at any time.
> 
> If you decide to play to this game, you have to use dladdr to check the
> origin of a symbol and try to guess whether it is what you are looking
> for or not.
> 
> Please dlopen(libSystem.dylib, RTLD_NOW) and search through it only.
> 
> 2. You shouldn't rely on return value of dlopen() or dlsym(). Call
> dlerror() and check whether it returns error or NULL.

I'll try.

> 
> PS:
>  What is the goal of using a get entropy ?

The function is rather new in the latest Solaris beta [1] and it's preferred 
to reading from /dev/random. There are already people suggest adding it to 
Linux. If I use simply using dlsym then it will automatically work on all 
current and future platforms. In fact, I was planning to write

getentropy = (GETENTROPY_FN)dlsym(RTLD_DEFAULT, "getentropy");
if (getentropy) {
return 1;
} else {
getrandom = (GETRANDOM_FN)dlsym(RTLD_DEFAULT, "getrandom");
if (getrandom) {
return 2;
} else {
return -1;  // will read /dev/random
}
}

Thanks
Max

[1] https://blogs.oracle.com/darren/entry/solaris_new_system_calls_getentropy

> 
> -Dmitry
> 
> On 2015-11-07 05:51, Wang Weijun wrote:
>> I find something strange.
>> 
>> Background: a new method getentropy() is available on OpenBSD [1] and 
>> Solaris and people are also proposing it on other OSes.
>> 
>> Therefore inside JDK I write a piece of native code to detect it, something 
>> like
>> 
>>typedef int (*GETENTROPY_FN)(char* buffer, int len);
>> 
>>getentropy = (GETENTROPY_FN)dlsym(RTLD_DEFAULT, "getentropy");
>>if (getentropy) {
>>return 1;
>>} 
>> 
>> On Mac, it returns non-NULL, but a later call to (*getentropy)(cbuffer, 
>> length) shows
>> 
>>  #  SIGBUS (0xa) at pc=0x000103bfa030, pid=22434, tid=5891
>>  ...
>>  # Problematic frame:
>>  # C  [libj2rand.dylib+0x1030]  getentropy+0x0
>> 
>> However, "man getentropy" does not show anything, and the following simple 
>> program also prints out 0x0
>> 
>> #include 
>> #include 
>> 
>> main() {
>>   void* g = dlsym(RTLD_DEFAULT, "getentropy");
>>   printf("%p\n", g);
>> }
>> 
>> What does this mean? Is the JDK code loading another getentropy() somewhere 
>> else? How do I detect it and what shall I do to avoid it?
>> 
>> Thanks
>> Max
>> 
>> [1] http://www.openbsd.org/cgi-bin/man.cgi/OpenBSD-current/man2/getentropy.2
>> 
> 
> 
> -- 
> Dmitry Samersoff
> Oracle Java development team, Saint Petersburg, Russia
> * I would love to change the world, but they won't give me the sources.

dlsym(RTLD_DEFAULT, "getentropy") return non-NULL on Mac

[Wang Weijun]

I find something strange.

Background: a new method getentropy() is available on OpenBSD [1] and Solaris 
and people are also proposing it on other OSes.

Therefore inside JDK I write a piece of native code to detect it, something like

typedef int (*GETENTROPY_FN)(char* buffer, int len);

getentropy = (GETENTROPY_FN)dlsym(RTLD_DEFAULT, "getentropy");
if (getentropy) {
return 1;
} 

On Mac, it returns non-NULL, but a later call to (*getentropy)(cbuffer, length) 
shows

  #  SIGBUS (0xa) at pc=0x000103bfa030, pid=22434, tid=5891
  ...
  # Problematic frame:
  # C  [libj2rand.dylib+0x1030]  getentropy+0x0

However, "man getentropy" does not show anything, and the following simple 
program also prints out 0x0

#include 
#include 

main() {
   void* g = dlsym(RTLD_DEFAULT, "getentropy");
   printf("%p\n", g);
}

What does this mean? Is the JDK code loading another getentropy() somewhere 
else? How do I detect it and what shall I do to avoid it?

Thanks
Max

[1] http://www.openbsd.org/cgi-bin/man.cgi/OpenBSD-current/man2/getentropy.2

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

[Wang Weijun]

Curious that you added a new method called 
jarFile.getRuntimeVersionedEntry(entryName). Is this the *only* method you 
would call for a multi-release jar? If so, is it still necessary to modify the 
old getEntry() method?

Thanks
Max

> On Nov 4, 2015, at 1:11 AM, Steve Drach  wrote:
> 
> Webrev: http://cr.openjdk.java.net/~psandoz/multiversion-jar/jar-webrev/
> 

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

[Wang Weijun]

> On Oct 21, 2015, at 3:17 PM, Xueming Shen  wrote:
> 
> We might want to bring in Max to take a look if what I said is really a 
> supported use scenario.

I haven't read Steve's latest code change. I will read if you think it's 
necessary.

First, I think we agree that the multi-release jar file feature is only making 
use of the existing jar file format and does not intend to introduce any change 
to its specification. This means a JarFile signed by one JDK release should be 
verified by another JDK release.

Ok, the next question is, should it modify the JarFile API? I hope not, because 
the JarFile API is the single entry we access a JarFile when we want to sign or 
verify it. I hope there is a brand new API for a multi-versioned jar file, 
probably a child class of JarFile, so that no matter you call getJarEntry() or 
entries() on it, you always get the versioned one and the unrelated ones are 
completely invisible.

If this is not OK, maybe we can rename the current JarFile to RawJarFile and 
name the new API JarFile. Signing and verification will work on RawJarFile.

Not sure if it's easy.

--Max

Re: Optional used as method argument?

[Wang Weijun]

> 在 2015年10月2日，下午9:51，Sean Mullan  写道：
> 
> 
> 
> On 10/2/15 9:27 AM, Wang Weijun wrote:
>> 
>>> 在 2015年10月2日，下午8:49，Roger Riggs  写道：
>>> 
>>> +1
>>> 
>>> The "no such value" makes me curious about the context.
>>> The @param tag really should be saying something about the parameter.
>> 
>> In fact, I'm working on a method which is similar to
>> 
>>   /*
>>* Generates some random bytes.
>>*
>>* @param n requested random value in bytes.
>>* @param extraEntropy optional entropy caller can provide, null if none.
>>*/
>>   byte[] getRandom(int n, byte[] extraEntropy)
>> 
>> Most caller won't be able to provide extra entropy, but if one does have a 
>> dedicated device which can generate fresh entropy, it is welcome.
> 
> Alternatively, you could just add an overloaded method, and not allow a null 
> value for extraEntropy:
> 
> byte[] getRandom(int n)
> byte[] getRandom(int n, byte[] extraEntropy)

Accepted.

Thanks
Max

> 
> --Sean

Re: Optional used as method argument?

[Wang Weijun]

> 在 2015年10月2日，下午8:49，Roger Riggs  写道：
> 
> +1
> 
> The "no such value" makes me curious about the context.
> The @param tag really should be saying something about the parameter.

In fact, I'm working on a method which is similar to

  /*
   * Generates some random bytes.
   *
   * @param n requested random value in bytes.
   * @param extraEntropy optional entropy caller can provide, null if none.
   */
  byte[] getRandom(int n, byte[] extraEntropy)

Most caller won't be able to provide extra entropy, but if one does have a 
dedicated device which can generate fresh entropy, it is welcome.

Thanks
Max

Optional used as method argument?

[Wang Weijun]

I hear people saying Optional is usually used as return values. Can I use it as 
an argument, like this?

  void consume(Optional value)

This way, I don't need to add spec like "@param value can be null if there is 
no such value".

Thanks
Max

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

[Wang Weijun]

> 在 2015年10月2日，上午12:15，Steve Drach  写道：
> 
> I think I’m getting distracted here, not focusing on getting tests created.  
> Is it okay to move on?

Please move on. If you think anything is strange, just send me all related 
files.

One last request about the SSLException, can you still generate the full debug 
output for jar signing with an empty cacerts? I think just adding a -debug 
option will print out a full stack trace showing where the exception is thrown.

Thanks
Max

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

[Wang Weijun]

> 在 2015年10月1日，下午7:36，Sean Mullan  写道：
> 
> 
> 
> On 10/1/15 5:10 AM, Wang Weijun wrote:
>> 
>>> 在 2015年10月1日，上午7:53，Steve Drach  写道：
>>> 
>>> - JDK 8 jar signer does not work with a JDK 9 created keystore
>>> - JDK 8 signed jar with JDK 8 created keystore is not the same size as JDK 
>>> 9 signed jar with JDK 9 keystore
>>> - JDK 8 signed jar with JDK 8 created keystore is not the same size as JDK 
>>> 9 signed jar with the same JDK 8 keystore
>> 
>> The default keystore type in jdk9 is pkcs12. I guess this is why jarsigner 
>> in jdk8 does not recognize it.
> 
> It should work if you are using 8u60. What JDK 8 version are you using?

Steve is using 8u51.

> 
> --Sean

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

[Wang Weijun]

> 在 2015年10月1日，上午7:53，Steve Drach  写道：
> 
> - JDK 8 jar signer does not work with a JDK 9 created keystore
> - JDK 8 signed jar with JDK 8 created keystore is not the same size as JDK 9 
> signed jar with JDK 9 keystore
> - JDK 8 signed jar with JDK 8 created keystore is not the same size as JDK 9 
> signed jar with the same JDK 8 keystore

The default keystore type in jdk9 is pkcs12. I guess this is why jarsigner in 
jdk8 does not recognize it.

As for the size, I guess the default signature algorithm or key size might not 
be the same, say, SHA1withRSA -> SHA256withRSA or 1024 to 2048.

--Max

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

[Wang Weijun]

> 在 2015年10月1日，上午8:21，Steve Drach  写道：
> 
>> Can you add a -debug option to show the full exception stack info? I even 
>> could not see how SSL is involved here.
> 
> Would you still like me to do this?

Yes, please. I cannot reproduce the problem in my jdk9 repo using a public 
cacerts file. If possible, can you also show me the jar file? Although I cannot 
figure out how special a jar file can be.

--Max

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

[Wang Weijun]

> 在 2015年10月1日，上午2:51，Sean Mullan  写道：
> 
>> The jarsigner from jdk9/dev can not, giving me the error
>> 
>> jarsigner: unable to sign jar: javax.net.ssl.SSLException: 
>> java.lang.RuntimeException: Unexpected error: 
>> java.security.InvalidAlgorithmParameterException: the trustAnchors parameter 
>> must be non-empty
>> 
>> I’m unsure what that means, and searching for it has not turned up anything 
>> useful except that it might be limited to Mac OS/X.  If anyone can help me 
>> here, I’d appreciate it.
> 
> This means it could not find a trusted root CA from the cacerts file to 
> validate the certificate chain. By default, OpenJDK includes an empty cacerts 
> file. You need to do a jdk9 build with the closed sources, as that is where 
> the trusted roots are.

If this is the problem, I think it's a bug. When jarsigner is signing it uses a 
key pair inside a keystone specified by -keystore. I don't see a reason why 
cacerts must be populated.

Can you add a -debug option to show the full exception stack info? I even could 
not see how SSL is involved here.

Thanks
Max

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

[Wang Weijun]

Can you describe if there is any effect on signed jars? Including:

1. Will jarsigner be able to sign such a jar? Are all items inside signed? If 
you sign a jar using jarsigner from different versions of JDK, will there be 
any difference?

2. Will jarsigner be able to verify such a jar? Will it only verify entries for 
the current version or all? Will jarsigner from an old JDK verify the new jar?

3. As I know, JarFile has 2 ways to verify a jar file, one using public APIs. 
One through SharedSecrets.setJavaUtilJarAccess() which can call more methods. 
Have you confirmed both work?

Yes, I'd also like some tests on these.

Thanks
Max

> On 2015年9月30日, at 上午2:13, Steve Drach  wrote:
> 
> Hi,
> 
> Please review the following webrev that adds support for multi-release jars 
> as specified in JEP-238.
> 
> Issue: https://bugs.openjdk.java.net/browse/JDK-8132734 
> 
> JEP 238: https://bugs.openjdk.java.net/browse/JDK-8047305 
> 
> Webrev: http://cr.openjdk.java.net/~psandoz/multiversion-jar/jar-webrev/ 
> 
> 
> A multi-release jar file is a jar file that contains a manifest with a main 
> attribute named "Multi-Release", and also contains a directory 
> "META-INF/versions" with subdirectories that contain versioned entries 
> segregated by the major version of Java platform releases. A versioned entry, 
> with a version n, in the "META-INF/versions/{n}" directory overrides the 
> unversioned root entry as well as any entry with a version number i  where i 
> < n.
> 
> The changes in this webrev implement an aliasing mechanism in JarEntry so 
> that when a JarFile client retrieves a JarEntry, the data from the entry 
> pointed to by the alias is returned.  There are methods to configure whether 
> or not aliasing is enabled, and if it is, which version of an entry the alias 
> points to.
> 
> When a JarFile is used by a class loader to load class resources, the default 
> version retrieved is the runtime version of the Java platform (i.e. a version 
> 9 entry is returned when the platform is JDK 9).  This mechanism can be 
> configured by System properties.
> 
> Thanks,
> Steve

Re: A PEM base64 decoder?

[Wang Weijun]

 
 In jdk8, we use Base64.getMimeDecoder() to parse PEM-encoded certs and it 
 ignores every character not in the base-64 alphabet. PEM is more 
 restricted and as I know openssl rejects PEM with illegal chars (Ex, "!" 
 as in bug report and test). This fix will also reject them.
>>> Shouldn't you add a Base64.getPemDecoder() with these semantics?  I
>>> think this decoder would be useful in other contexts as well.
>> Sherman, is that possible?
>> 
> 
> While it is possible personally I will be a little hesitated to add the 
> support for a
> "deprecated" rfc into the "new" Base64 class. Any evidence that PEM is still
> heavily used in other contexts?

Not sure about usage outside the security area, it's heavily used for encoding 
of certificates, CRLs, private keys, etc.

I did some experiments, creating a PEM certificate including garbage 
characters, only Firefox accepts it, and it's rejected by IE, openssl, Mac.

--Max

> 
> -Sherman

A PEM base64 decoder? (was: RFR 8074935: jdk8 keytool doesn't validate pem files for RFC 1421 correctness, as jdk7 did)

[Wang Weijun]

> Begin forwarded message:
> 
> Date: March 23, 2015 at 16:33:18 GMT+8
> From: Florian Weimer 
> To: Wang Weijun , OpenJDK Dev list 
> 
> Subject: Re: RFR 8074935: jdk8 keytool doesn't validate pem files for RFC 
> 1421 correctness, as jdk7 did
> 
> On 03/17/2015 11:02 AM, Wang Weijun wrote:
>> Hi All
>> 
>> Please review the code change at
>> 
>>   http://cr.openjdk.java.net/~weijun/8074935/webrev.00/
>> 
>> In jdk8, we use Base64.getMimeDecoder() to parse PEM-encoded certs and it 
>> ignores every character not in the base-64 alphabet. PEM is more restricted 
>> and as I know openssl rejects PEM with illegal chars (Ex, "!" as in bug 
>> report and test). This fix will also reject them.
> 
> Shouldn't you add a Base64.getPemDecoder() with these semantics?  I
> think this decoder would be useful in other contexts as well.

Sherman, is that possible?

Thanks
Max

> 
> -- 
> Florian Weimer / Red Hat Product Security

Re: FilePermission Canonical path optimization

[Wang Weijun]

> On Feb 9, 2015, at 14:42, Peter Levart  wrote:
> 
> Hi Max,
> 
> Of course you are aware that by trusting the symlinks, you potentially give 
> much more permission than you would hope to. Suppose that some code has 
> permission to read and write into a particular directory (for temporary 
> files). With this permission the code can actually read and/or write any file 
> in the filesystem that OS grants access to the java process. Merely by 
> creating a symlink in the read/write-able directory and accessing the file 
> through it. That's why Apache HTTP Server by default disables 
> "FollowSymLinks" option.

Yes, we will be careful.

In Java, a LinkPermission is needed to create a link. Of course, there might be 
other (existing) symlinks created by other non-Java processes. We will evaluate 
this possibility.

Thanks
Max

Re: FilePermission Canonical path optimization

[Wang Weijun]

> On Feb 9, 2015, at 13:27, deven you  wrote:
> 
> Hi Weijun,
> 
> From my understanding, the new proposal will let implies method only depends 
> on the absolute path in policy file, correct? So it's user's responsibility 
> to ensure files who want to access is relative to the absolute path in some 
> policy file?

No, you can still add a FilePermission on a relative path, and then it only 
allows you accessing the file with a relative path.

For example, if the current working directory is /home/me, and the policy file 
has

   FilePermission doc/-, read;

You can only call new FileInputStream("doc/a.txt"), you cannot call new 
FileInputStream("/home/me/doc/a.txt"), because without consulting the file 
system (i.e. canonicalize the path), there is no way to find out 
/home/me/doc/a.txt is inside doc.

On the other hand, if the policy file has

   FilePermission /etc/passwd, read;

You cannot call new FileInputStream("../../etc/passwd"), although we think 
nobody will try that.

> 
> I personal agree this proposal. Is there any doc or link for this new 
> proposal? Or if you can update the information for this proposal here, I will 
> be very appreciate!

Not yet. This is just an experiment, and given the incompatibility, we are 
still evaluating if it is doable. As I said in my previous mail, we don't want 
anyone to rewrite his/her apps, and we hope it's easy to modify policy files.

Actually, since this makes FilePermission simpler, there won't be a long doc.

Thanks
Max

> 
> Thanks a lot!
> 
> 2015-02-09 11:51 GMT+08:00 Wang Weijun :
> 
> > On Feb 9, 2015, at 11:22, deven you  wrote:
> >
> > Hi Weijun,
> >
> > I see JDK-4141872 marked as Not an Issue, is there any further task 
> > continue, or there is any link else to track this problem to remove the 
> > canonical path?
> 
> It was marked as Not an Issue, but we are reconsidering about it.
> 
> >
> > It's a big improvement if canonical path can be totally removed but I can't 
> > figure out how we get the result of the implies* methods without canonical 
> > path? Any more detail?
> 
> The current proposed idea is that if you want to access a file using absolute 
> path, you should add a FilePermission line in the policy file with an 
> absolute path. If relative, relative. The overall idea is that the implies 
> method should be implemented without consulting the actual file system but 
> only by looking at the names themselves.
> 
> That's why I said there is a very big incompatible change. We hope people 
> only needs to modify their policy files and do not need to rewrite their 
> apps, but we are still investigating if this can always be true.
> 
> Thanks
> Max
> 
> >
> > Thanks a lot!
> 
> 

Re: FilePermission Canonical path optimization

[Wang Weijun]

> On Feb 9, 2015, at 11:22, deven you  wrote:
> 
> Hi Weijun,
> 
> I see JDK-4141872 marked as Not an Issue, is there any further task continue, 
> or there is any link else to track this problem to remove the canonical path?

It was marked as Not an Issue, but we are reconsidering about it.

> 
> It's a big improvement if canonical path can be totally removed but I can't 
> figure out how we get the result of the implies* methods without canonical 
> path? Any more detail?

The current proposed idea is that if you want to access a file using absolute 
path, you should add a FilePermission line in the policy file with an absolute 
path. If relative, relative. The overall idea is that the implies method should 
be implemented without consulting the actual file system but only by looking at 
the names themselves.

That's why I said there is a very big incompatible change. We hope people only 
needs to modify their policy files and do not need to rewrite their apps, but 
we are still investigating if this can always be true.

Thanks
Max

> 
> Thanks a lot!

Re: FilePermission Canonical path optimization

[Wang Weijun]

Hi Deven

Sorry for the noise, but in fact we are looking into removing the 
canonicalization step because of

 4141872: FilePermission makes symlinks useless
 https://bugs.openjdk.java.net/browse/JDK-4141872

This will be a very big incompatible change and we are still doing a 
feasibility study.

Of course, we won't touch jdk8u or earlier.

Thanks
Max

> On Feb 6, 2015, at 14:09, deven you  wrote:
> 
> Hi All,
> 
> I have updated the patch[1] according to above discussion. Please review it.
> 
> Thanks a lot
> 
> [1] http://cr.openjdk.java.net/~youdwei/ojdk-912/webrev.03/
> 

Re: [9] request for review: 8049171: Additional tests for jarsigner's warnings

[Wang Weijun]

JarUtils:

You can break after line 83.

Otherwise very good.

Thanks
Max

> On Jan 26, 2015, at 15:55, Artem Smotrakov  wrote:
> 
> Hi Max,
> 
> Here is an updated webrev, please take a look.
> 
> http://cr.openjdk.java.net/~asmotrak/8049171/webrev.02/
> 
> Artem
> 
> On 01/26/2015 05:03 AM, Weijun Wang wrote:
>> 
>> 
>> On 1/23/2015 16:12, Artem Smotrakov wrote:
>>> If the MANIFEST and the signature files must be at the beginning, should
>>> it be considered as a bug in jarsigner? Should it reject such files?
>> 
>> I think so. Will file a bug.
>> 
 
 The "jar u" way is to copy each old entry into destination unless the
 entry name is in the updated list where the new file will be read.
 Finally the untouched files in the updated list are appended.
>>> Since tests were not originally for checking some unusual ways for
>>> updating jars, I think they need to be updated to use the "jar u" way
>>> for adding unsigned entry.
>> 
>> Good.
>> 
>> Thanks
>> Max
>> 
>>> 
>>> Artem
 
>>> 
>>> 
>>> 
> 

Re: [9] request for review: 8049171: Additional tests for jarsigner's warnings

[Wang Weijun]

> On Jan 22, 2015, at 19:40, Artem Smotrakov  wrote:
> 
>> I am not sure if I understand updateJar correctly. It looks like srcJarFile 
>> is opened multiple times so its entries are duplicated a lot in the 
>> destination. Or is there a secret break?
> There is no any secret, just a bug. It is not necessary to open srcJarFile 
> multiple times.
> 
> I have updated the webrev, updateJar() method does the following:
> - creates a new jar file (destJarFilename)
> - puts files which are specified in files parameter to destJarFilename
> - copies files from srcJarFilename to destJarFilename if they are no files 
> with the same names in destJarFilename

The process above means the new files are added at the beginning. While 
jarsigner is able to verify such a file (it uses JarFile) the output is 
actually invalid because the MANIFEST and the signature files must be at the 
beginning (otherwise a JarInputStream cannot verify it).

The "jar u" way is to copy each old entry into destination unless the entry 
name is in the updated list where the new file will be read. Finally the 
untouched files in the updated list are appended.

> 
> Here is an updated webrev with your suggested changes and a couple of others:
> - added @ignore tag for BadKeyUsageTest since it fails due to a bug in JDK
> - updated MultipleWarningsTest test to check ExtendedKeyUsage case instead of 
> KeyUsage

Great.

Thanks
Max

> 
> http://cr.openjdk.java.net/~asmotrak/8049171/webrev.01/
> 

Re: [9] request for review: 8049171: Additional tests for jarsigner's warnings

[Wang Weijun]

Thanks for adding so many tests. Some suggestions:

- JarUtils.java

You can use the new InputStream.transferTo() method.

I am not sure if I understand updateJar correctly. It looks like srcJarFile is 
opened multiple times so its entries are duplicated a lot in the destination. 
Or is there a secret break?

- Utils.java

The mixed using of File and Files is strange, but you are free to keep it.

- TimestampCheck.java

You can make Handler Autocloseable to use try-with-resources in tests.

- Various tests

I am not a fan of calling Utils.cleanup() in final block. Unless you have 
created huge garbages, those files are precious when the test fails (given you 
provide -retain in jtreg, which I always do).

Thanks
Max

> On Jan 21, 2015, at 14:35, Artem Smotrakov  wrote:
> 
> Hello,
> 
> Please review a couple of new tests for jarsigner's warnings. Basically tests 
> run jarsigner and check warning/error messages and exit codes according to 
> [1].
> 
> https://bugs.openjdk.java.net/browse/JDK-8049171
> http://cr.openjdk.java.net/~asmotrak/8049171/webrev.00
> 
> [1] 
> http://docs.oracle.com/javase/7/docs/technotes/tools/windows/jarsigner.html
> 
> Artem
> 
> 

Re: RFR: 8067951: System.loadLibrary cannot find library when path contains quoted entry

[Wang Weijun]

> On Dec 23, 2014, at 21:31, Ivan Gerasimov  wrote:
> 
> BUGURL: https://bugs.openjdk.java.net/browse/JDK-8067951
> WEBREV: http://cr.openjdk.java.net/~igerasim/8067951/0/webrev/

I am not sure why you want to rewrite the loop entirely. Isn't it OK to just 
add that long if check before the old line 1770? I always find myself confusing 
when a for line contains "," inside. :-)

And how about a white box test with 
getDeclaredMethod("initializePath").setAccessible(true)? I think you can even 
add a few non-supported case (say, "A:B":C) and comment them out at the moment.

Thanks
Max

Copy a ZipEntry into another zip file

[Wang Weijun]

Hi Sherman

The jarsigner tool contains these 2 methods to copy a ZipEntry into the signer 
jar:

private void writeEntry(ZipFile zf, ZipOutputStream os, ZipEntry ze)
throws IOException
{
ZipEntry ze2 = new ZipEntry(ze.getName());
ze2.setMethod(ze.getMethod());
ze2.setTime(ze.getTime());
ze2.setComment(ze.getComment());
ze2.setExtra(ze.getExtra());
if (ze.getMethod() == ZipEntry.STORED) {
ze2.setSize(ze.getSize());
ze2.setCrc(ze.getCrc());
}
os.putNextEntry(ze2);
writeBytes(zf, ze, os);
}

/**
 * Writes all the bytes for a given entry to the specified output stream.
 */
private synchronized void writeBytes
(ZipFile zf, ZipEntry ze, ZipOutputStream os) throws IOException {
int n;

InputStream is = null;
try {
is = zf.getInputStream(ze);
long left = ze.getSize();

while((left > 0) && (n = is.read(buffer, 0, buffer.length)) != -1) {
os.write(buffer, 0, n);
left -= n;
}
} finally {
if (is != null) {
is.close();
}
}
}

Several questions:

1. Why cannot I just call os.putNextEntry(ze) or at least os.putNextEntry(new 
ZipEntry(ze))? Maybe some fields (say, compressed size) should not be copied 
over? If ze2 must be this way, shall I also copy the flag field?

2. In writeBytes(), why does the getSize() return value need to be used? 
Shouldn't we just transfer all bytes from is to os?

Thanks
Max

Re: Should some JDK system properties be read only ?

[Wang Weijun]

A System.setFinalProperty() method that creates a new property with a final 
value? Maybe also a System.isFinalProperty() method allowing people to detect 
if a property is final.

--Max

Re: FilePermission Canonical path optimization

[Wang Weijun]

Do you need some kind of synchronization on the get_dir_rec() method?

--Max

> On Dec 1, 2014, at 16:06, deven you  wrote:
> 
> Hi All,
> File.getCanonicalPath() is a very time-consuming method, we observed
> significant performance degradation from some application's startup stage
> with java.io.FilePermission. However, lazying load the calls to
> getCanonicalPath() from java.ioFilePermission is straightforward and solve
> this problem effectively. Openjdk bug[1]  tracks this bug and here is the
> patch [2]. Could anyone take a look?
> 
> [1] https://bugs.openjdk.java.net/browse/JDK-8066211
> [2] http://cr.openjdk.java.net/~youdwei/ojdk-912/webrev.00/

Re: Review request: JDK-8055723 Replace concat String to append in StringBuilder parameters

[Wang Weijun]

I hope we can restrict the code change to what the bug description is about. 
IMHO this bug should only include cleanup and introduce no obvious behavior 
change.

Any other fix can go to another bug.

--Max

> On Nov 13, 2014, at 08:57, Otávio Gonçalves de Santana  
> wrote:
> 
> But this class is an Exception, doesn't make sense an exception get another
> Exception.
> IMHO: I prefer this way
> 
> On Wed, Nov 12, 2014 at 8:36 AM, Ulf Zibis  wrote:
> 
>> Hi Otávio,
>> I now think you could replace
>> if (!expected.isEmpty())
>> with
>> assert !expected.isEmpty();
>> 
>> If expected ever would be empty, the only thing which happens is, that a
>> "'" is missing in a message which anyway doesn't make sense without
>> arguments.
>> 
>> -Ulf
>> 
>> 
..

Re: How to extract matches from (\d+[hms])+ ?

[Wang Weijun]

On Sep 25, 2014, at 13:06, Guy Steele  wrote:

> (A lurker sticking his nose in here! :-)  Is it your intent also to match 
> "30s1h" or "20m30m" as a time duration?
> 
> If not, you might be better off with a pattern such as 
> "((\\d+)h)?((\\d+)m)?((\\d+)s)?" and then the whole problem caused by the 
> outer "+" iteration disappear (but you may need to check whether the original 
> string was empty).

Yes, this is much better.

> 
> But maybe that takes all the fun out of it.

Let someone else enjoy it then. :-)

Thanks
Max

> 
> --Guy Steele
> 
> On Sep 25, 2014, at 12:51 AM, Wang Weijun  wrote:
> 
>> Hi Sherman
>> 
>> I want to match a time duration like "1h20m30s" and "2h". It looks like if I 
>> directly use the pattern "((\\d+)([hms]))+", group(2) and group (3) only 
>> return the last match (i.e. 30 and s for 1h20m30s). So I tried multiple 
>> matching with "(\\d)([hms])" only, but find() does not always match from the 
>> beginning, and lookingAt() does not advance after one call.
>> 
>> This is my code now;
>> 
>> int start = 0;
>> while (true) {
>> if (!m.find() || m.start() != start) {
>>   throw new Exception();
>> }
>> start = m.end();
>> print(m.group(1), m.group(2));
>> if (m.hitEnd()) break;
>> }
>> print("Done");
>> 
>> Is this the correct way?
>> 
>> Thanks
>> Max
>> 
> 

How to extract matches from (\d+[hms])+ ?

[Wang Weijun]

Hi Sherman

I want to match a time duration like "1h20m30s" and "2h". It looks like if I 
directly use the pattern "((\\d+)([hms]))+", group(2) and group (3) only return 
the last match (i.e. 30 and s for 1h20m30s). So I tried multiple matching with 
"(\\d)([hms])" only, but find() does not always match from the beginning, and 
lookingAt() does not advance after one call.

This is my code now;

int start = 0;
while (true) {
  if (!m.find() || m.start() != start) {
throw new Exception();
  }
  start = m.end();
  print(m.group(1), m.group(2));
  if (m.hitEnd()) break;
}
print("Done");

Is this the correct way?

Thanks
Max

Re: Replace concat String to append in StringBuilder parameters

[Wang Weijun]

So it's not that the optimization fails but there is no optimization on them 
yet.

I do see the .append("x") case will be easy to deal with, but it looks like 
historically javac has not been a place to do many optimizations. It mostly 
converts the java source to byte codes in a 1-to-1 mapping and let VM do 
whatever it wants (to optimize). When you talked about compiling multiple 
concatenation into using a single StringBuilder, it's more like choosing the 
correct implementation rather than an optimization.

I don't expect to see big change on this in the near future, so shall we go on 
with the current enhancement?

Thanks
Max

On Aug 29, 2014, at 2:17, Ulf Zibis  wrote:

> I mean:
> It does not output byte code that only uses a single char array to compose 
> the entire String in question.
> With "optimization fails", I also mean, there is used an additional 
> "StringComposer" e.g. another StringBuilder or a StringJoiner in addition to 
> the 1st StringBuilder.
> 
> -Ulf
> 
> Am 27.08.2014 um 14:02 schrieb Pavel Rappo:
>> Could you please explain what you mean by "javac optimization fails" here?
>> 
>> -Pavel
>> 
>> On 27 Aug 2014, at 10:41, Ulf Zibis  wrote:
>> 
>>> 4.) Now we see, that javac optimization fails again if StringBuilder, 
>>> concatenation, toString(), append(String), append(Collection) etc. and 
>>> StringJoiner use is mixed.
>> 
> 

Re: Replace concat String to append in StringBuilder parameters

[Wang Weijun]

OK, I'll remember that. So you will include the StringBuilder changes into your 
fix?

--Max

On Aug 28, 2014, at 2:10, Ivan Gerasimov  wrote:

> Hi Max!
> 
>> The core part is updated again at
>> 
>>   http://cr.openjdk.java.net/~weijun/8055723/core/webrev.03/
> 
> Can you please revert changes to 
> src/java.base/share/classes/sun/net/www/MimeEntry.java, as they're 
> conflicting with the fix for JDK-8054714?
> 
> Apologizing for adding you more work.
> 
> Sincerely yours,
> Ivan
> 

Re: Replace concat String to append in StringBuilder parameters

[Wang Weijun]

On Aug 27, 2014, at 10:07, Wang Weijun  wrote:

> Webrev updated again, this time include more changes.
> 
>  http://cr.openjdk.java.net/~weijun/8055723/client/webrev.02/
>  http://cr.openjdk.java.net/~weijun/8055723/core/webrev.02/

The core part is updated again at

  http://cr.openjdk.java.net/~weijun/8055723/core/webrev.03/

including a fix s/(int)c/c/ in com/sun/tools/hat/internal/util/Misc.java and 
literal string merge in X509CRLEntryImpl.java, X509CRLImpl.java, and 
X509CertInfo.java.

No change for the client part.

Thanks
Max

Re: Replace concat String to append in StringBuilder parameters

[Wang Weijun]

I found an error:

diff --git 
a/src/jdk.dev/share/classes/com/sun/tools/hat/internal/util/Misc.java 
b/src/jdk.dev/share/classes/com/sun/tools/hat/internal/util/Misc.java
--- a/src/jdk.dev/share/classes/com/sun/tools/hat/internal/util/Misc.java
+++ b/src/jdk.dev/share/classes/com/sun/tools/hat/internal/util/Misc.java
@@ -97,11 +97,11 @@
 } else if (ch == '&') {
 sb.append("&");
 } else if (ch < ' ') {
-sb.append("&#" + Integer.toString(ch) + ";");
+sb.append("&#").append((int)ch).append(';');
 } else {
 int c = (ch & 0x);
 if (c > 127) {
-sb.append("&#" + Integer.toString(c) + ";");
+sb.append("&#").append((int)c).append(';');
 } else {
 sb.append(ch);
 }

In the 2nd change, it should be "append(c)" instead of "append((int)c)". Here c 
is already an integer and this redundant cast will be treated as an error when 
-Werror specified.

--Max

On Aug 27, 2014, at 10:07, Wang Weijun  wrote:

> Webrev updated again, this time include more changes.
> 
>  http://cr.openjdk.java.net/~weijun/8055723/client/webrev.02/
>  http://cr.openjdk.java.net/~weijun/8055723/core/webrev.02/
> 
> The change to a demo file is removed because that file itself is already 
> removed.
> 
> *Otávio*: I believe Andrej's following suggestion is worth looking at.
> 
> Thanks
> Max
> 
> On Aug 26, 2014, at 19:12, Andrej Golovnin  wrote:
> 
>> 
>> src/java.base/share/classes/sun/security/x509/X509CRLEntryImpl.java:
>> 316 sb.append("Extension unknown: " + "DER 
>> encoded OCTET string =\n")
>> 
>> should be changed to:
>> 316 sb.append("Extension unknown: DER encoded 
>> OCTET string =\n")
>> 
>> same problems are in 
>> src/java.base/share/classes/sun/security/x509/X509CRLImpl.java in the line 
>> 576
>> src/java.base/share/classes/sun/security/x509/X509CertInfo.java in the line 
>> 332
>> 
>> Best regards,
>> Andrej Golovnin
> 

Re: Replace concat String to append in StringBuilder parameters

[Wang Weijun]

Webrev updated again, this time include more changes.

  http://cr.openjdk.java.net/~weijun/8055723/client/webrev.02/
  http://cr.openjdk.java.net/~weijun/8055723/core/webrev.02/

The change to a demo file is removed because that file itself is already 
removed.

*Otávio*: I believe Andrej's following suggestion is worth looking at.

Thanks
Max

On Aug 26, 2014, at 19:12, Andrej Golovnin  wrote:

> 
> src/java.base/share/classes/sun/security/x509/X509CRLEntryImpl.java:
>  316 sb.append("Extension unknown: " + "DER 
> encoded OCTET string =\n")
> 
> should be changed to:
>  316 sb.append("Extension unknown: DER encoded 
> OCTET string =\n")
> 
> same problems are in 
> src/java.base/share/classes/sun/security/x509/X509CRLImpl.java in the line 576
> src/java.base/share/classes/sun/security/x509/X509CertInfo.java in the line 
> 332
> 
> Best regards,
> Andrej Golovnin

Re: Replace concat String to append in StringBuilder parameters

[Wang Weijun]

I see no problem from the core part of the webrev.

However, I am not sure how you find all the occurrences of "+" in 
StringBuilder, but I just run the following command in jdk/src

  find . -type f -name *.java -print | xargs grep -n StringBuilder | perl -ne 
'print if /new StringBuilder\([^\)]*\+/'
  find . -type f -name *.java -print | xargs grep -n append | perl -ne 'print 
if /append\([^\)]*\+/'

and there are still many results.

Some are false alarms like

  buf.append(" + ");
  result.append(patt.charAt(i + 1));

but I still see

  sb.append("Extension unknown: " + "DER encoded OCTET string =\n")
  StringBuilder sb = new StringBuilder("\"" + ti.getThreadName() + "\"" +
  tagBuffer.append("");
  ...

Thanks
Max


On Aug 26, 2014, at 11:28, Wang Weijun  wrote:

> New webrevs available at
> 
>  http://cr.openjdk.java.net/~weijun/8055723/client/webrev.01/
>  http://cr.openjdk.java.net/~weijun/8055723/core/webrev.01/  
> 
> There are only 2 now. Everything non-client is in core.
> 
> Everyone, please do code review quickly because the patch touches too many 
> files and any delay could mean re-merge.
> 
> *Otávio*: If there is only small change in feedback, tell me to update my own 
> repo and you don't need to generate the big patch again.
> 
> I see you still include that demo file.
> 
> Thanks
> Max
> 
> 

Re: Replace concat String to append in StringBuilder parameters

[Wang Weijun]

New webrevs available at

  http://cr.openjdk.java.net/~weijun/8055723/client/webrev.01/
  http://cr.openjdk.java.net/~weijun/8055723/core/webrev.01/  

There are only 2 now. Everything non-client is in core.

Everyone, please do code review quickly because the patch touches too many 
files and any delay could mean re-merge.

*Otávio*: If there is only small change in feedback, tell me to update my own 
repo and you don't need to generate the big patch again.

I see you still include that demo file.

Thanks
Max

Re: Replace concat String to append in StringBuilder parameters

[Wang Weijun]

New webrevs updated

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

  Includes modules java.base and security-related modules and the jarsigner tool

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

  Includes the java.desktop module

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

  Includes java.corba, java.management, java.naming, java.rmi, jdk.dev, 
jdk.jcmd, jdk.jconsole, jdk.jdi, jdk.jvmstat, /jdk.naming.dns, jdk.runtime, 
jdk.snmp, and a jpda demo file

Thanks
Max
 

Re: Trusted service?

[Wang Weijun]

On Aug 13, 2014, at 23:31, mark.reinh...@oracle.com wrote:

> 2014/8/13 7:20 -0700, alan.bate...@oracle.com:
>> The usual thing is to just have a default implementation that is used 
>> when ServiceLoader doesn't locate a useful provider. You'll find many 
>> examples of this in the JDK. In those cases then the default is not 
>> listed in a services configuration file. From what you describe then 
>> this may be what you want too.
>> 
>> ServiceLoader does not have a way to configure a preferred provider so 
>> this is one reason why you'll see places where a system property can be 
>> used to configured the preferred implementation.
> 
> Another alternative is to use the ServiceLoader::loadInstalled method,
> which will ignore providers on the application class path.

Great, this works for me.

But why does it need to be called in a doPrivileged() block? Isn't it only 
about JDK-internal classes/resources?

Thanks
Max

> 
> - Mark

Re: Replace concat String to append in StringBuilder parameters

[Wang Weijun]

I also see a lot of .toString() and String.valueOf() calls.

$ cat string_concat_updated.patch | perl -ne 'print if /^\+ 
.*append.*(String\.valueOf|\.toString\(\))/' | wc
  62 2104626

Wrapped lines not indented correctly in

src/java.xml.crypto/share/classes/com/sun/org/apache/xml/internal/security/encryption/AbstractSerializer.java
src/java.base/share/classes/com/sun/crypto/provider/DHParameters.java
src/java.base/share/classes/com/sun/crypto/provider/DHPublicKey.java
src/java.base/share/classes/com/sun/crypto/provider/GCMParameters.java

--Max

Re: Replace concat String to append in StringBuilder parameters

[Wang Weijun]

On Aug 21, 2014, at 21:18, Andrej Golovnin  wrote:

>https://bugs.openjdk.java.net/browse/JDK-8038277
> 
> This is not the right bug report. The subject of this bug report is "Improve 
> the bootstrap performance of carets keystore".

Oh, my mistake, it should be https://bugs.openjdk.java.net/browse/JDK-8055723. 
Will update it in the next round of webrev.

>  
> 
>http://cr.openjdk.java.net/~weijun/8038277/client/webrev.00
> 
> TABs are still used for indentation. I looked only at 
> src/java.desktop/share/classes/javax/swing/RepaintManager.java.

My mistake again. In order to create 3 changesets for a single bug id, I turned 
off jcheck.

These files includes TABs:

client:

Warning: src/java.desktop/share/classes/javax/swing/GroupLayout.java:1242: Tab 
character
Warning: src/java.desktop/share/classes/javax/swing/MultiUIDefaults.java:193: 
Tab character
Warning: src/java.desktop/share/classes/javax/swing/RepaintManager.java:993: 
Tab character

core:

Warning: src/java.base/share/classes/java/security/ProtectionDomain.java:288: 
Tab character
Warning: src/java.base/share/classes/sun/launcher/LauncherHelper.java:372: Tab 
character
Warning: src/java.base/share/classes/sun/net/www/HeaderParser.java:224: Tab 
character
Warning: 
src/java.base/share/classes/sun/security/provider/PolicyFile.java:1488: Tab 
character
Warning: 
src/java.base/share/classes/sun/security/ssl/SupportedEllipticCurvesExtension.java:115:
 Tab character

extra:

Warning: 
src/java.management/share/classes/java/lang/management/ThreadInfo.java:582: Tab 
character
Warning: src/jdk.dev/share/classes/sun/security/tools/jarsigner/Main.java:707: 
Tab character
Warning: 
src/jdk.jconsole/share/classes/sun/tools/jconsole/inspector/XArrayDataViewer.java:99:
 Tab character
Warning: 
src/jdk.naming.dns/share/classes/sun/net/spi/nameservice/dns/DNSNameService.java:494:
 Tab character

Thanks
Max

Re: Replace concat String to append in StringBuilder parameters

[Wang Weijun]

I filed a bug at

   https://bugs.openjdk.java.net/browse/JDK-8038277

Webrev in 3 parts at

   http://cr.openjdk.java.net/~weijun/8038277/client/webrev.00
   http://cr.openjdk.java.net/~weijun/8038277/core/webrev.00/
   http://cr.openjdk.java.net/~weijun/8038277/extra/webrev.00/

--Max

On Aug 21, 2014, at 10:32, Otávio Gonçalves de Santana  
wrote:

> Thank you Wang.
> Actually I haven't neither webrev and bug id.
> I believe is better split in client and server code.
> https://dl.dropboxusercontent.com/u/16109193/open_jdk/string_builder_concat_8.zip
> 
> 
> On Tue, Aug 19, 2014 at 10:55 PM, Wang Weijun  wrote:
> Hi Otávio
> 
> I see TABs in the first page of sun_security.diff, too long line in 
> javax_security.diff.
> 
> Also, it's unfortunate that you will need to rename the file names to the new 
> style with modules. See 
> http://cr.openjdk.java.net/~chegar/docs/portingScript.html for how to do this.
> 
> I can create webrev page(s) for you on cr.openjdk.java.net. Please tell me if 
> you want a big one or one for each diff.
> 
> I see no bug id. If none, I can create one for you.
> 
> Thanks
> Max
> 
> On Aug 20, 2014, at 9:05, Otávio Gonçalves de Santana  
> wrote:
> 
> > Thank you Sergey.
> > https://dl.dropboxusercontent.com/u/16109193/open_jdk/string_builder_concat_7.zip
> >
> >
> > On Tue, Aug 19, 2014 at 12:32 PM, Sergey Bylokhov <
> > sergey.bylok...@oracle.com> wrote:
> >
> >> Hi Otávio,
> >> The new alignment in DataLine.java/JColorChooser.java looks strange.
> >> Wrong change in BasicTableUI.java:
> >> -plainStr.deleteCharAt(plainStr.length() -
> >> 1).append("\n");
> >> +plainStr.deleteCharAt(plainStr.length() -
> >> 1).append('\t');
> >>
> >>
> >> On 13.08.2014 3:01, Otávio Gonçalves de Santana wrote:
> >>
> >>> Thank you Roger.
> >>> Done
> >>> https://dl.dropboxusercontent.com/u/16109193/open_jdk/
> >>> string_builder_concat_6.zip
> >>>
> >>>
> >>> On Tue, Aug 12, 2014 at 10:15 AM, roger riggs 
> >>> wrote:
> >>>
> >>> fyi,
> >>>>
> >>>> There's a Perl script normalizer.pl that detects/fixes most of the
> >>>> simple
> >>>> tab/white space issues.
> >>>> The script is in the /make/scripts/normalizer.pl
> >>>>
> >>>> Roger
> >>>>
> >>>>
> >>>> On 8/12/2014 3:48 AM, Andrej Golovnin wrote:
> >>>>
> >>>> Hi Otávio,
> >>>>>
> >>>>> I think you should fix the indentation in a lot of classes. You use the
> >>>>> tab-character for the indentation. As far as I know we should use the
> >>>>> space
> >>>>> character for the indentation in the JDK sources (Oracle devs feel free
> >>>>> to
> >>>>> correct me if I'm wrong. And it would be really nice if the style guide
> >>>>> for
> >>>>> the source code would be a part of the JDK repository. So we don't need
> >>>>> to
> >>>>> search for it on the internet/wiki. Just clone the repository, read the
> >>>>> style guide and follow it. :-) ). Here is the not complete list of
> >>>>> classes
> >>>>> where you used the tab-character for the indentation:
> >>>>>
> >>>>> src/share/classes/com/sun/crypto/provider/OAEPParameters.java
> >>>>> src/share/classes/java/lang/management/MemoryUsage.java
> >>>>> src/share/classes/java/security/KeyStore.java
> >>>>> src/share/classes/java/security/PermissionCollection.java
> >>>>> src/share/classes/java/security/ProtectionDomain.java
> >>>>> src/share/classes/java/security/cert/CertPath.java
> >>>>> src/share/classes/java/security/cert/PKIXCertPathBuilderResult.java
> >>>>> src/share/classes/java/security/cert/PKIXParameters.java
> >>>>> src/share/classes/java/security/cert/PolicyQualifierInfo.java
> >>>>> src/share/classes/java/security/cert/TrustAnchor.java
> >>>>> src/share/classes/java/security/cert/X509CertSelector.java
> >>>>> src/share/classes/javax/crypto/CryptoPermission.java
> >>>>> src/share/classes/javax/management/relation/Role.java
> >>>>>
> >>>>>
> >>>>> In src/share/classes/com/sun/jmx/snmp/IPAcl/Parser.jj in the line 423 a
> >>>>> dot
> >>>>> is missed before append:
> >>>>>
> >>>>> 423   {jjtn000.name.append( '.')append(t.image); }
> >>>>>
> >>>>> Best regards,
> >>>>> Andrej Golovnin
> >>>>>
> >>>>>
> >>>>
> >>>
> >>
> >> --
> >> Best regards, Sergey.
> >>
> >>
> >
> >
> > --
> > Otávio Gonçalves de Santana
> >
> > blog: http://otaviosantana.blogspot.com.br/
> > twitter: http://twitter.com/otaviojava
> > site: *http://about.me/otaviojava <http://about.me/otaviojava>*
> > 55 (11) 98255-3513
> > 
> 
> 
> 
> 
> -- 
> Otávio Gonçalves de Santana
> 
> blog: http://otaviosantana.blogspot.com.br/
> twitter: http://twitter.com/otaviojava
> site: http://about.me/otaviojava
> 55 (11) 98255-3513
> 

Re: Replace concat String to append in StringBuilder parameters

[Wang Weijun]

Hi Otávio

I see TABs in the first page of sun_security.diff, too long line in 
javax_security.diff.

Also, it's unfortunate that you will need to rename the file names to the new 
style with modules. See 
http://cr.openjdk.java.net/~chegar/docs/portingScript.html for how to do this.

I can create webrev page(s) for you on cr.openjdk.java.net. Please tell me if 
you want a big one or one for each diff.

I see no bug id. If none, I can create one for you.

Thanks
Max

On Aug 20, 2014, at 9:05, Otávio Gonçalves de Santana  
wrote:

> Thank you Sergey.
> https://dl.dropboxusercontent.com/u/16109193/open_jdk/string_builder_concat_7.zip
> 
> 
> On Tue, Aug 19, 2014 at 12:32 PM, Sergey Bylokhov <
> sergey.bylok...@oracle.com> wrote:
> 
>> Hi Otávio,
>> The new alignment in DataLine.java/JColorChooser.java looks strange.
>> Wrong change in BasicTableUI.java:
>> -plainStr.deleteCharAt(plainStr.length() -
>> 1).append("\n");
>> +plainStr.deleteCharAt(plainStr.length() -
>> 1).append('\t');
>> 
>> 
>> On 13.08.2014 3:01, Otávio Gonçalves de Santana wrote:
>> 
>>> Thank you Roger.
>>> Done
>>> https://dl.dropboxusercontent.com/u/16109193/open_jdk/
>>> string_builder_concat_6.zip
>>> 
>>> 
>>> On Tue, Aug 12, 2014 at 10:15 AM, roger riggs 
>>> wrote:
>>> 
>>> fyi,
 
 There's a Perl script normalizer.pl that detects/fixes most of the
 simple
 tab/white space issues.
 The script is in the /make/scripts/normalizer.pl
 
 Roger
 
 
 On 8/12/2014 3:48 AM, Andrej Golovnin wrote:
 
 Hi Otávio,
> 
> I think you should fix the indentation in a lot of classes. You use the
> tab-character for the indentation. As far as I know we should use the
> space
> character for the indentation in the JDK sources (Oracle devs feel free
> to
> correct me if I'm wrong. And it would be really nice if the style guide
> for
> the source code would be a part of the JDK repository. So we don't need
> to
> search for it on the internet/wiki. Just clone the repository, read the
> style guide and follow it. :-) ). Here is the not complete list of
> classes
> where you used the tab-character for the indentation:
> 
> src/share/classes/com/sun/crypto/provider/OAEPParameters.java
> src/share/classes/java/lang/management/MemoryUsage.java
> src/share/classes/java/security/KeyStore.java
> src/share/classes/java/security/PermissionCollection.java
> src/share/classes/java/security/ProtectionDomain.java
> src/share/classes/java/security/cert/CertPath.java
> src/share/classes/java/security/cert/PKIXCertPathBuilderResult.java
> src/share/classes/java/security/cert/PKIXParameters.java
> src/share/classes/java/security/cert/PolicyQualifierInfo.java
> src/share/classes/java/security/cert/TrustAnchor.java
> src/share/classes/java/security/cert/X509CertSelector.java
> src/share/classes/javax/crypto/CryptoPermission.java
> src/share/classes/javax/management/relation/Role.java
> 
> 
> In src/share/classes/com/sun/jmx/snmp/IPAcl/Parser.jj in the line 423 a
> dot
> is missed before append:
> 
> 423   {jjtn000.name.append( '.')append(t.image); }
> 
> Best regards,
> Andrej Golovnin
> 
> 
 
>>> 
>> 
>> --
>> Best regards, Sergey.
>> 
>> 
> 
> 
> -- 
> Otávio Gonçalves de Santana
> 
> blog: http://otaviosantana.blogspot.com.br/
> twitter: http://twitter.com/otaviojava
> site: *http://about.me/otaviojava *
> 55 (11) 98255-3513
> 

Re: Trusted service?

[Wang Weijun]

On Aug 14, 2014, at 16:03, Alan Bateman  wrote:
> 
>> Or writing the class name in a services file automatically exports it as a 
>> service?
>> 
>> Now my preferred order will be
>> 
>>if (loadProviderFromProperty())
>>return provider;
>>if (loadProviderAsInstalledService())
>>return provider;
>>if (loadProviderAsService())
>>return provider;
>> 
> Looking at JDK-8038089 again then I'm not sure why loadProviderFromProperty 
> is here. If you are limiting this to just implementations of the Kerberos 
> cipher suites and you only want to use service providers that are in the JDK 
> image then ServiceLoader.loadInstalled should be sufficient.

That's just a general preferred order. Here I define no property and do not 
expect an application providing an impl so yes loadProviderAsInstalledService 
is enough.

--Max

> 
> -Alan

How to use jvisualvm to find memory leaks?

[Wang Weijun]

I am looking at "8054896: Loading a KeyStore prevents GC of Classloader" and am 
now able to reproduce it on my system. I do see the webapp-related classes 
present in a heapdump created by VisualVM but not sure what the next steps 
should be to tell why they are not GC'ed. Is there a good tutorial on this?

Thanks
Max

Re: Trusted service?

[Wang Weijun]

Yes, I see a lot of places using

   if (loadProviderFromProperty())
   return provider;
   if (loadProviderAsService())
   return provider;

The 1st using Class.forName() and 2nd ServiceLoader.load().

I was thinking that the 1st method will not work because Class.forName() on an 
internal class in another module will not work after module, but now I believe 
the 2nd method would not work also. Is that right? Or writing the class name in 
a services file automatically exports it as a service?

Now my preferred order will be

   if (loadProviderFromProperty())
   return provider;
   if (loadProviderAsInstalledService())
   return provider;
   if (loadProviderAsService())
   return provider;

Thanks
Max


On Aug 13, 2014, at 22:20, Alan Bateman  wrote:

> 
> ServiceLoader does not have a way to configure a preferred provider so this 
> is one reason why you'll see places where a system property can be used to 
> configured the preferred implementation.
> 
> -Alan.

Trusted service?

[Wang Weijun]

Hi All

I'm working on "8038089: TLS optional support for Kerberos cipher suites needs 
to be re-examine" which will separate the implementation of Kerberos-related 
TLS ciphersuites from the other TLS codes. I am thinking of defining a 
ServiceLoader interface called ExternalCipherSuiteProvider inside the TLS 
module and implement a Krb5CipherSuiteProvider in the JGSS module. Now if the 
JGSS module is installed, it will be found and thus supports the TLS_KRB5_* 
ciphersuites.

However, it looks like any application can include an implementation and 
register it by adding its own $CLASSPATH/META-INF/services line. Is there 
anyway I can find out which is the "trusted" one? I've looked at some 
ServiceLoader example inside JDK and it looks like they first load an 
implementation specified by a system property and then do the 
ServiceLoader.load() loop. Is that system property meant to provide the 
"trusted" or "builtin" implementation? I wonder if it still works now because 
even if we define a system property (or security property), the implementation 
class will be invisible in a different module.

Thanks
Max

Re: Replace concat String to append in StringBuilder parameters

[Wang Weijun]

No TAB, no \r, and no trailing space are hard requirements enforced by jcheck. 
Otherwise it's only styles, including 4-space-indentation. "{" at the end of a 
line, 8-space wrap indentation...

--Max (an Oracle dev)

On Aug 12, 2014, at 15:48, Andrej Golovnin  wrote:

> As far as I know we should use the space character for the indentation in the 
> JDK sources (Oracle devs feel free to correct me if I'm wrong.

Re: Replace concat String to append in StringBuilder parameters

[Wang Weijun]

'\"' can be written as '"':

com_sun.diff:209:+sb.append(' 
').append(nodeName).append("=\"").append(att.getNodeValue()).append('\"');
java_lang.diff:31:+ sb.append('\"').append(getThreadName()).append('\"')
java_security.diff:78:+.append('\"');
sun_security.diff:95:+
sb.append(principalInfo[i][0]).append(' 
').append('\"').append(principalInfo[i][1]).append('\"');
sun_security.diff:108:+.append('\"');
sun_security.diff:122:+sb.append(X500PRINCIPAL).append(" 
\"").append(suffix).append('\"');
sun_security.diff:312:+
retval.append('\"').append(sbuffer.toString()).append('\"');

Still using append("."):

java_security.diff:107:+sb.append("\n").append(type).append(" Cert 
Path: length = ").append(getCertificates().size()).append(".\n");
java_security.diff:148:+sb.append("  serverName: 
").append(serverName).append("\n");
javax_swing.diff:79:+sb.append(getClass().getName()).append(" 
").append(Integer.toString(hashCode()));
sun_security.diff:419:+sb.append("X.509 CRL v").append(version + 
1).append("\n");

I only read the security related files, but I grep for the two groups above.

Also, it's better to put rb.getString(...) to one line

sun_security.diff:268:+
sb.append('\n').append(tab).append(rb.getString(
sun_security.diff:273:+
sb.append('\n').append(tab).append(rb.getString(

And some lines are too long.

Thanks
Max
  
On Aug 11, 2014, at 11:29, Otávio Gonçalves de Santana  
wrote:

> Done.
> 
> https://dl.dropboxusercontent.com/u/16109193/open_jdk/string_builder_concat_2.zip
> 
> obs: stay the 2 chars to better performance.
> 
> 
> On Sun, Aug 10, 2014 at 8:03 PM, Claes Redestad  
> wrote:
> +1
> 
> Some suggestions (mostly nits):
> 
> - in places like src/share/classes/java/util/regex/Pattern.java you 
> introducesingle-char
>   strings which might use a char instead:
> 
> -result.append("|"+next);
> +result.append('|').append(next);
> 
> - in places like src/share/classes/sun/security/provider/PolicyFile.java
>   you end up with a sequence of String literal appends which could be merged 
> into one:
> 
> -sb.append(principalInfo[i][0] + " " +
> -"\"" + principalInfo[i][1] + "\"");
> +sb.append(principalInfo[i][0]).append(" \"")
> +.append(principalInfo[i][1]).append('"');
> 
> - in some places like src/share/classes/java/text/ChoiceFormat.java
>   you end up doing append(""). I guess the empty string concatenation was 
> used as a form
>   of primitive-to-string coercion and was probably always redundant already, 
> but care needs
>   to be taken that doing the append directly behave as intended.
> 
>   I think it should be safe to just remove the empty string append in most 
> cases:
> 
> -result.append(""+choiceLimits[i]);
> +result.append(choiceLimits[i]);
> 
> Thanks!
> 
> /Claes
> 
> On 2014-08-10 23:33, Otávio Gonçalves de Santana wrote:
> *Motivation:* Make another append instead of concat String inside of append
> 
> parameter in StringBuilder class. To avoid an extra StringBuilder created
> for the purpose of concatenating. So it will save memory and will faster
> than concat String.
> Doing a code to benchMark[1], the result is:
> 
> Benchmark  Mode   Samples
>Mean   Mean errorUnits
> m.StringBuilderConcatBenchMark.stringBuilder  thrpt10
>   6317444.705   108673.584ops/s
> m.StringBuilderConcatBenchMark.stringBuilderWithConcatthrpt10
>   3354554.43568353.924ops/s
> 
> The webrev of all code is:
> https://dl.dropboxusercontent.com/u/16109193/open_jdk/string_builder_concat.zip
> 
> 
> [1]
> 
> @State(Scope.Thread)
> @OutputTimeUnit(TimeUnit.SECONDS)
> public class StringBuilderConcatBenchMark {
> 
> 
>  private static final String F = "";
>   private static final String E = " running in Java ";
> private static final String D = " in the code ";
>   private static final String C = " to try impact ";
> private static final String B = " with some text ";
>   private static final String A = "Doing a test";
> 
> @GenerateMicroBenchmark
> public void stringBuilder(BlackHole bh) {
>   bh.consume(createBuilder(A, B, C, D, E, F));
> }
> 
> @GenerateMicroBenchmark
>   public void stringBuilderWithConcat(BlackHole bh) {
> bh.consume(createBuilderWithConcat(A, B, C, D, E, F));
>   }
> 
> private StringBuilder createBuilder(String... values) {
> StringBuilder text = new StringBuilder();
>   text.append(values[0]).append(values[1])
> .append(values[2]).append(values[3])
> .append(values[4]).append(values[5]);
>   return text;
> }
>   private StringBuilder createBui

Re: RFR 8054095: No space allowed in platforms string in ProblemList.txt

[Wang Weijun]

The change looks good. So does the closed side.

I read jtharness and it seems a line is broken into 3 parts separated by 
space(s). Either the 2nd or the 3rd part will be used by jtreg as platforms, 
but not both. I'm not sure if jtharness or jtreg should also be updated.

Thanks
Max


On Aug 1, 2014, at 14:45, Amy Lu  wrote:

> Please review the problem list change for JDK-8054095
> 
> http://cr.openjdk.java.net/~ewang/amylu/JDK-8054095/webrev.00/
> 
> In ProblemList.txt, when specify multiple platforms, the space between the 
> platform names is not accepted by jtreg harness.
> This change is to remove the empty space between the platform names.
> Thanks,
> Amy
> 

Re: JEP 198: Light-Weight JSON API

[Wang Weijun]

On Jul 31, 2014, at 8:35, Remi Forax  wrote:

> 
> On 07/25/2014 04:45 PM, mark.reinh...@oracle.com wrote:
>> New JEP Candidate: http://openjdk.java.net/jeps/198
>> 
>> - Mark
> 
> Hi Mark, Hi Mike,
> Implementing a json API was one of the use case I've used during the 
> development of the lambdas,
> so maybe you are interested by this gist
>  https://gist.github.com/forax/81a56cf2684bfa2e46ec

I am reading "[ \"foo\", \"bar\" ]" and feel we need a JEP for new styles of 
literal strings. Long long ago there was a proposal for multi-line raw strings. 
Still alive?

Thanks
Max