Re: RFR(XXS): 8205916: [test] Fix jdk/tools/launcher/RunpathTest to handle both, RPATH and RUNPATH

Hi Martin, Erik, thanks for the reviews! Regards, Volker On Wed, Jun 27, 2018 at 5:53 PM, Erik Joelsson wrote: > Looks ok to me. > > /Erik > > > > On 2018-06-27 03:26, Volker Simonis wrote: >> >> Hi, >> >> can I please have a review for the following tiny test fix (I'm >> actually not sure whi

Re: Draft JEP proposal: JDK-8200758: Packaging Tool

Hi Kevin, Just a small request, can you call it something like jbundler and not jpackager, because usually in build tools the packager step is the step that create the jars, not the one that bundle the whole application ? regards, Rémi - Mail original - > De: "Kevin Rushforth" > À: "co

Re: [PATCH] AbstractStringBuilder.append()

I'm fairly sure the append(StringBuilder) overloads were left out intentionally. On Wed, Jun 27, 2018 at 8:57 PM, Isaac Levy wrote: > AbstractStringBuilder already has append(). This patch > adds append(). > > The new method improves parity between the two classes. In addition, > combining Strin

[PATCH] AbstractStringBuilder.append()

AbstractStringBuilder already has append(). This patch adds append(). The new method improves parity between the two classes. In addition, combining StringBuilders is presumably common. append() has a couple insteadof checks, which this new method skips. -Isaac diff --git a/src/java.base/shar

[JDK 11] RFR 8187069: The case auto failed with the "java.lang.ClassNotFoundException: IPv6NameserverPlatformParsingTest" exception

Please review below one line change for manual test case com/sun/jndi/dns/Test6991580.java to build test class automatically which will be used in manual steps, thanks bug: https://bugs.openjdk.java.net/browse/JDK-8187069 Review change as belo

Re: Draft JEP proposal: JDK-8200758: Packaging Tool

We're aiming to get this into JDK 12 early enough so that an EA build would be available around the time JDK 11 ships. That will allow you to take a jlinked image with JDK 11 and package it up using (the EA) jpackager. We will create a development branch in the JDK sandbox [1] some time in the

Re: RFR: 8205438: Re-enable shebang tests in test/jdk/tools/launchers/SourceMode.java

We went through the same exercise with jjs. Tough coming up with shebang tests that worked on all platforms. Shebang args vs command line args was a nightmare. Sent from my iPhone > On Jun 27, 2018, at 6:02 PM, mandy chung wrote: > > Looks good. It's amazing to find out 3 different behavior

Re: RFR: 8205438: Re-enable shebang tests in test/jdk/tools/launchers/SourceMode.java

Looks good. It's amazing to find out 3 different behavior on these 3 platforms. Mandy On 6/27/18 1:37 PM, Jonathan Gibbons wrote: Please review a test fix to re-enable shebang tests in test/jdk/tools/launchers/SourceMode.java . The test cases for invoking the source launcher via the shebang

RFR: 8205438: Re-enable shebang tests in test/jdk/tools/launchers/SourceMode.java

Please review a test fix to re-enable shebang tests in test/jdk/tools/launchers/SourceMode.java . The test cases for invoking the source launcher via the shebang mechanism have been disabled because they were adversely affected by the very long pathnames on our internal test infastructure, cau

Re: RFR: 8148188: Enhance the security libraries to record events of interest

Finished.java - In each handshake, Finished messages are delivered twice. One from client, and the other one from the server side. Catch Finished message and use the final one of them should be sufficient. In the Finished.java implementation, the signal of the final Finished mes

Re: RFR: 8148188: Enhance the security libraries to record events of interest

On 2018-06-27 21:14, Seán Coffey wrote: On 27/06/2018 19:57, Xuelei Fan wrote: Hi Sean, I may reply in several replies. PKIXMasterCertPathValidator.java + CertChainEvent cce = new CertChainEvent(); + if(cce.isEnabled() || EventHelper.loggingSecurity()) { +

Re: RFR: 8148188: Enhance the security libraries to record events of interest

KeyUtil.java 167 public static String getKeyAlgorithm(Key key) { I may suggest remove this method and use Key.getAlgorithm() directly. The KeyUtil.getKeyAlgorithm() is incomplete, and may need additional maintenance if new key algorithms are added in the future. For example, in

Re: RFR: 8148188: Enhance the security libraries to record events of interest

On 27/06/2018 19:57, Xuelei Fan wrote: Hi Sean, I may reply in several replies. PKIXMasterCertPathValidator.java +  CertChainEvent cce = new CertChainEvent(); +  if(cce.isEnabled() || EventHelper.loggingSecurity()) { +  String c = reversedCertList.stream(

Re: RFR: 8148188: Enhance the security libraries to record events of interest

Hi Sean, I may reply in several replies. PKIXMasterCertPathValidator.java + CertChainEvent cce = new CertChainEvent(); + if(cce.isEnabled() || EventHelper.loggingSecurity()) { + String c = reversedCertList.stream() + .map(x -> x.getSerialN

Re: RFR(JDK11/NIO) 8205058 throw CharacterCodingException --> Re: RFR (JDK11/NIO) 8201276: (fs) Add methods to Files for reading/writing a string from/to a file

Thanks all! Glad to be able to get this done right before the deadline :-) -Joe On 6/27/18, 12:44 AM, Alan Bateman wrote: On 26/06/2018 20:49, Joe Wang wrote: : Removed the null check. The internal impl and test guarantees it's not null indeed: http://cr.openjdk.java.net/~joehw/jdk11/82050

Re: RFR(XXS): 8205916: [test] Fix jdk/tools/launcher/RunpathTest to handle both, RPATH and RUNPATH

Looks ok to me. /Erik On 2018-06-27 03:26, Volker Simonis wrote: Hi, can I please have a review for the following tiny test fix (I'm actually not sure which would be the appropriate mailing list for this fix so please redirect if necessary): http://cr.openjdk.java.net/~simonis/webrevs/2018/8

Re: RFR JDK-8200243: System error message is decoded as invalid encoding in Windows.

On 6/27/18, 12:39 AM, Alan Bateman wrote: On 27/06/2018 04:14, Xueming Shen wrote: Hi, Please help review the change for JDK-8200243 issue: https://bugs.openjdk.java.net/browse/JDK-8200243 webrev: http://cr.openjdk.java.net/~sherman/8200243/webrev This is a regression. The root cause is one o

Re: RFR(XXS): 8205916: [test] Fix jdk/tools/launcher/RunpathTest to handle both, RPATH and RUNPATH

Looks good to me! On Wed, Jun 27, 2018 at 3:26 AM, Volker Simonis wrote: > Hi, > > can I please have a review for the following tiny test fix (I'm > actually not sure which would be the appropriate mailing list for this > fix so please redirect if necessary): > > http://cr.openjdk.java.net/~simo

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

Hi Sean, Updated webrev: http://cr.openjdk.java.net/~rriggs/webrev-static-property-8066709/index.html I added the suggested text and updated the class level warning to match. Thanks, Roger On 6/27/2018 6:58 AM, Sean Mullan wrote: I think it is worth putting a stronger warning in each of the m

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

I rechecked, almost all were within doPriv and the others have new explicit SM checks. Thanks, Roger On 6/27/2018 1:54 AM, mandy chung wrote: Looks good to me.  The part that I'm unsure is whether you caught all the callers that are outside doPrivileged to StaticProperty.* will need to do its

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

I think it is worth putting a stronger warning in each of the methods (and not just the class description) of StaticProperty that additional care should be taken when using these methods since there is no SecurityManager check. For example: "{@link SecurityManager#checkPropertyAccess} is NOT

RFR(XXS): 8205916: [test] Fix jdk/tools/launcher/RunpathTest to handle both, RPATH and RUNPATH

Hi, can I please have a review for the following tiny test fix (I'm actually not sure which would be the appropriate mailing list for this fix so please redirect if necessary): http://cr.openjdk.java.net/~simonis/webrevs/2018/8205916/ https://bugs.openjdk.java.net/browse/JDK-8205916 The test cur

Re: Draft JEP proposal: JDK-8200758: Packaging Tool

Thanks for the info! And thanks for the efforts. [no irony, no sarcasm - I really mean it] But, to sum up my comprehension... anyone who placed their bets on javapackager, starting with last LTS Java 8 will be left in the rain with followup LTS Java 11, because their ain't neither javapackager

Re: RFR(JDK11/NIO) 8205058 throw CharacterCodingException --> Re: RFR (JDK11/NIO) 8201276: (fs) Add methods to Files for reading/writing a string from/to a file

On 26/06/2018 20:49, Joe Wang wrote: : Removed the null check. The internal impl and test guarantees it's not null indeed: http://cr.openjdk.java.net/~joehw/jdk11/8205058/webrev04/ Looks good. -Alan

Re: RFR JDK-8200243: System error message is decoded as invalid encoding in Windows.

On 27/06/2018 04:14, Xueming Shen wrote: Hi, Please help review the change for JDK-8200243 issue: https://bugs.openjdk.java.net/browse/JDK-8200243 webrev: http://cr.openjdk.java.net/~sherman/8200243/webrev This is a regression. The root cause is one of the change we put in jdk9 for JDK-80577

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

On 27/06/2018 03:10, Roger Riggs wrote: Hi, Updated webrev: http://cr.openjdk.java.net/~rriggs/webrev-static-property-8066709/index.html Applied changes from prior comments and droped a change no longer needed due to the TLS 1.3 removal of ClientKeyExchangeService.java. The CSR has been