Re: RFR 8213031: (zipfs) Add support for POSIX file permissions

2018-11-05 Thread Alan Bateman
On 05/11/2018 07:32, Langer, Christoph wrote: Hi, Ping. May I get reviews/substantial feedback on this zipfs enhancement? It might be bit early to be asking for a code review on just one piece of this. I think the first step on this feature has to be to put all the issues on the table.

Re: RFR 8213031: (zipfs) Add support for POSIX file permissions

2018-11-05 Thread Chris Hegarty
Hi Christoph, On 05/11/18 07:32, Langer, Christoph wrote: .. CSR: https://bugs.openjdk.java.net/browse/JDK-8213082 Can you please add a `Scope` value to the CSR? I can't quite tell, but I assume it should be `JDK` or `Implementation`, right? I want to clarify that there is no impact on Java

RE: RFR 8213031: (zipfs) Add support for POSIX file permissions

2018-11-05 Thread Langer, Christoph
Hi Chris > The reason I asked about the CSR scope clarification is that it was > unclear to me what the ultimate intentions are, given that the previous > mails ( linked to from the CSR ) did have Java SE API changes ( in the > java.util.zip package ). > > Are you now happy to reduce the scope

RE: RFR 8213031: (zipfs) Add support for POSIX file permissions

2018-11-05 Thread Langer, Christoph
Hi Chris, yes, there's no impact on Java SE with this item. No API is changed. I've set the scope to JDK, as it affects the features that are available with the "jar" filesystem provider from module jdk.zipfs. Best regards Christoph > -Original Message- > From: Chris Hegarty > Sent:

RE: RFR 8213031: (zipfs) Add support for POSIX file permissions

2018-11-05 Thread Langer, Christoph
Hi Alan, all, I'd welcome a discussion, for sure. Unfortunately there hasn't been so much participation in this yet. I think this is an item where it's hard to have a clear opinion and where it's difficult to oversee all implications it might have. Who'd be willing to have a look from

Re: RFR 8213031: (zipfs) Add support for POSIX file permissions

2018-11-05 Thread Chris Hegarty
On 05/11/18 12:54, Langer, Christoph wrote: Hi Chris, yes, there's no impact on Java SE with this item. No API is changed. I've set the scope to JDK, as it affects the features that are available with the "jar" filesystem provider from module jdk.zipfs. ... The reason I asked about the

Re: RFR(M) 8212605: Pure-Java implementation of AccessController.doPrivileged

2018-11-05 Thread Roger Riggs
Hi Dean, typo AccessController line788: "annocations" The implementations of doPrivileged(PrivilegedExceptionAction action) and doPrivileged(PrivilegedAction action) Could be a bit more similar since except for the exception wrapping they are the same. 309 return executePrivileged(action,

Re: RFR 8213031: (zipfs) Add support for POSIX file permissions

2018-11-05 Thread Volker Simonis
Hi Christoph, in general your change looks good to me. I need some more time to think about the implications mentioned by Alan and Chris but for the time being please find some comments regarding your implementation below: ZipFileAttributeView.java - can you please throw an AssertionError() for

Re: RFR(M) 8212605: Pure-Java implementation of AccessController.doPrivileged

2018-11-05 Thread dean . long
Thanks Alan. dl On 11/4/18 1:03 AM, Alan Bateman wrote: On 03/11/2018 20:00, dean.l...@oracle.com wrote: I made a pass at improving the comments based on feedback I've received.  I updated webrev.4 in place, along with an incremental diff: I looked through the updated webrev.4, mostly

Re: RFR[s] 8211339: NPE during SSL handshake caused by HostnameChecker

2018-11-05 Thread Xuelei Fan
It looks fine to me. Thanks, Xuelei On 11/5/2018 10:59 AM, Anthony Scarpino wrote: Hi, I'd like to get a code review of this simple change.  It's a simple check to make sure the hostname variable is not null and throw a proper exception.

Re: RFR 8213031: (zipfs) Add support for POSIX file permissions

2018-11-05 Thread Chris Hegarty
On 05/11/18 15:59, Alan Bateman wrote: On 05/11/2018 13:05, Langer, Christoph wrote: ... I think you'll need to do a write-up of the overall proposal so that folks can jump in and point out the implications. It's not easy to do this in a code review of a small piece of the solution.

Re: A new proposal to add methods to HttpsURLConnection to access SSLSession

2018-11-05 Thread Xuelei Fan
Hi Chris and Sean, There are a few feedback for the CSR approval. I updated to use Optional for the returned value. For more details, please refer to: https://bugs.openjdk.java.net/browse/JDK-8213161 http://cr.openjdk.java.net/~xuelei/8212261/webrev.03/ Please let me know if you are

RFR[s] 8211339: NPE during SSL handshake caused by HostnameChecker

2018-11-05 Thread Anthony Scarpino
Hi, I'd like to get a code review of this simple change. It's a simple check to make sure the hostname variable is not null and throw a proper exception. http://cr.openjdk.java.net/~ascarpino/8211339/webrev.00/ Tony

Re: RFR 8213031: (zipfs) Add support for POSIX file permissions

2018-11-05 Thread Alan Bateman
On 05/11/2018 13:05, Langer, Christoph wrote: Hi Alan, all, I’d welcome a discussion, for sure. Unfortunately there hasn’t been so much participation in this yet. I think this is an item where it’s hard to have a clear opinion and where it’s difficult to oversee all implications it might

Re: RFR(M) 8212605: Pure-Java implementation of AccessController.doPrivileged

2018-11-05 Thread Roger Riggs
Hi Dean, Looks ok, I have no better suggestion. Roger On 11/05/2018 01:51 PM, dean.l...@oracle.com wrote: Hi Roger.  Thanks for looking at this. On 11/5/18 7:21 AM, Roger Riggs wrote: Hi Dean, typo AccessController line788: "annocations" Fixed. The implementations of

Re: RFR[12] JDK-8211049 "Second parameter of "initialize" method is not used"

2018-11-05 Thread Valerie Peng
Sure, I did test only against SunRsaSign provider in my initial version if that's what you prefer. Valerie On 11/1/2018 7:55 PM, Weijun Wang wrote: Hi Valerie Since you are specifically fixing a bug inside the SunRsaSign provider, is it enough to just use the KeyPairGenerator from this

Re: RFR[12] JDK-8211049 "Second parameter of "initialize" method is not used"

2018-11-05 Thread Valerie Peng
Webrev updated at: http://cr.openjdk.java.net/~valeriep/8211049/webrev.01 Thanks, Valerie On 11/5/2018 11:52 AM, Valerie Peng wrote: Sure, I did test only against SunRsaSign provider in my initial version if that's what you prefer. Valerie On 11/1/2018 7:55 PM, Weijun Wang wrote: Hi

Re: RFR[s] 8211339: NPE during SSL handshake caused by HostnameChecker

2018-11-05 Thread Norman Maurer
LGTM as well… thanks a lot! > On 5. Nov 2018, at 20:22, Xuelei Fan wrote: > > It looks fine to me. > > Thanks, > Xuelei > > On 11/5/2018 10:59 AM, Anthony Scarpino wrote: >> Hi, >> I'd like to get a code review of this simple change. It's a simple check to >> make sure the hostname

Re: RFR CSR for 8213400: Support choosing curve name in keytool keypair generation

2018-11-05 Thread Xuelei Fan
On 11/5/2018 7:13 PM, Weijun Wang wrote: Please take a review at the CSR at https://bugs.openjdk.java.net/browse/JDK-8213401 As for implementation, I intend to report an error when -keyalg is not EC but -curvename is provided. If both -curvename and -keysize are provided, I intend to

Re: RFR[12] JDK-8211049 "Second parameter of "initialize" method is not used"

2018-11-05 Thread Valerie Peng
Thanks for the review~ Valerie On 11/5/2018 4:30 PM, Weijun Wang wrote: Hi Valerie The change looks fine to me. Thanks Max On Nov 6, 2018, at 6:26 AM, Valerie Peng wrote: Webrev updated at: http://cr.openjdk.java.net/~valeriep/8211049/webrev.01 Thanks, Valerie On 11/5/2018 11:52 AM,

Re: RFR CSR for 8213400: Support choosing curve name in keytool keypair generation

2018-11-05 Thread Xuelei Fan
On 11/5/2018 8:37 PM, Weijun Wang wrote: On Nov 6, 2018, at 12:12 PM, Xuelei Fan wrote: On 11/5/2018 7:13 PM, Weijun Wang wrote: Please take a review at the CSR at https://bugs.openjdk.java.net/browse/JDK-8213401 As for implementation, I intend to report an error when -keyalg is not EC

RFR CSR for 8213400: Support choosing curve name in keytool keypair generation

2018-11-05 Thread Weijun Wang
Please take a review at the CSR at https://bugs.openjdk.java.net/browse/JDK-8213401 As for implementation, I intend to report an error when -keyalg is not EC but -curvename is provided. If both -curvename and -keysize are provided, I intend to ignore -keysize no matter if they match or not.

Re: RFR CSR for 8213400: Support choosing curve name in keytool keypair generation

2018-11-05 Thread Weijun Wang
> On Nov 6, 2018, at 12:12 PM, Xuelei Fan wrote: > > On 11/5/2018 7:13 PM, Weijun Wang wrote: >> Please take a review at the CSR at >>https://bugs.openjdk.java.net/browse/JDK-8213401 >> As for implementation, I intend to report an error when -keyalg is not EC >> but -curvename is

Re: RFR(M) 8212605: Pure-Java implementation of AccessController.doPrivileged

2018-11-05 Thread dean . long
Thanks Roger. dl On 11/5/18 2:00 PM, Roger Riggs wrote: Hi Dean, Looks ok, I have no better suggestion. Roger On 11/05/2018 01:51 PM, dean.l...@oracle.com wrote: Hi Roger.  Thanks for looking at this. On 11/5/18 7:21 AM, Roger Riggs wrote: Hi Dean, typo AccessController line788:

Re: RFR[12] JDK-8211049 "Second parameter of "initialize" method is not used"

2018-11-05 Thread Weijun Wang
Hi Valerie The change looks fine to me. Thanks Max > On Nov 6, 2018, at 6:26 AM, Valerie Peng wrote: > > Webrev updated at: http://cr.openjdk.java.net/~valeriep/8211049/webrev.01 > > Thanks, > > Valerie > > > On 11/5/2018 11:52 AM, Valerie Peng wrote: >> >> Sure, I did test only against

java.lang.NoSuchFieldError: state

2018-11-05 Thread Martin Choma
Hey, with upgrade to 1.8.0_191 I hit "java.lang.NoSuchFieldError: state" during SSL handshake. I have found https://bugs.java.com/view_bug.do?bug_id=JDK-8074462 but I couldn find openjdk counterpart. Some more details can be found [1]. Regards, Martin Choma [1]

Re: RFR CSR for 8213400: Support choosing curve name in keytool keypair generation

2018-11-05 Thread Bernd Eckenfels
Hello, I would agree ignoring an (conflicting) option adds confusion. When specifying a curve is a new feature we don’t need to worry about beeing compatible, therefore I would forbid mixing curve names and keysize at all (even when the size matches). I guess we cannot remove the option to

RFR, JDK-8212885: TLS 1.3 resumed session does not retain peer certificate chain

2018-11-05 Thread Jamil Nimeh
Hello all, This fixes an issue where TLS 1.3 resumed sessions were not carrying forward many of the parameters from the parent session, namely the peer certificates, but also the local certificates and a few other SSLSessionImpl fields.  This also moves the fix from an earlier, related issue

Re: java.lang.NoSuchFieldError: state

2018-11-05 Thread Norman Maurer
I think this may be caused by using the wrong jetty alpn version. Be sure you use the correct one. For example 2.0.9: https://github.com/jetty-project/jetty-alpn-agent/releases Bye Norman > On 6. Nov 2018, at 07:46, Martin Choma

Re: RFR CSR for 8213400: Support choosing curve name in keytool keypair generation

2018-11-05 Thread Weijun Wang
> On Nov 6, 2018, at 1:06 PM, Xuelei Fan wrote: > > On 11/5/2018 8:37 PM, Weijun Wang wrote: >>> On Nov 6, 2018, at 12:12 PM, Xuelei Fan wrote: >>> >>> On 11/5/2018 7:13 PM, Weijun Wang wrote: Please take a review at the CSR at https://bugs.openjdk.java.net/browse/JDK-8213401