Re: [9] RFR: 8073430: Deprecate security APIs that have been superseded
What is the policy for deprecating a non-public-API method? > On Mar 6, 2015, at 13:30, Wang Weijun wrote: > > Instead of adding @SupressWarnings to a method, can we add @Deprecated to it?
Re: [9] RFR: 8073430: Deprecate security APIs that have been superseded
Instead of adding @SupressWarnings to a method, can we add @Deprecated to it? If I understand correctly, there should be no warning if a deprecated method calls another deprecated method. And by deprecating the caller we will get warnings if they are called by more methods and we can evaluate if they can be further deprecated or modified. This way we would reveal everything that is related with the deprecated APIs. Hopefully they are actually useless inside JDK. --Max > On Mar 6, 2015, at 12:01, Jason Uh wrote: > > Hi Max, > > A couple of these, we probably won't be able to remove in JDK 9. I'm > deprecating getPeerCertificateChain() in the javax.net.ssl.SSLSession > interface in this change, so the implementation in > sun.security.ssl.SSLSessionImpl will have to be suppressed. Also, > X509V1CertImpl will probably have to be marked with @SupressWarnings in JDK 9 > and then hopefully it can be removed altogether in JDK 10 along with > javax.security.cert. > > As for some of the other methods causing warnings, I think they can actually > be removed, but I'd like to track that change in a different issue. I'm not > sure yet, but there might have to be some minor test changes to accommodate > the changes, too. I filed an issue to track it: > https://bugs.openjdk.java.net/browse/JDK-8074531 > > For now, here are my changes again with Sean's suggested changes to the > package-info.java files. > http://cr.openjdk.java.net/~juh/8073430/01/ > > Thanks, > Jason
Re: [9] RFR: 8073430: Deprecate security APIs that have been superseded
Hi Max,
A couple of these, we probably won't be able to remove in JDK 9. I'm
deprecating getPeerCertificateChain() in the javax.net.ssl.SSLSession
interface in this change, so the implementation in
sun.security.ssl.SSLSessionImpl will have to be suppressed. Also,
X509V1CertImpl will probably have to be marked with @SupressWarnings in
JDK 9 and then hopefully it can be removed altogether in JDK 10 along
with javax.security.cert.
As for some of the other methods causing warnings, I think they can
actually be removed, but I'd like to track that change in a different
issue. I'm not sure yet, but there might have to be some minor test
changes to accommodate the changes, too. I filed an issue to track it:
https://bugs.openjdk.java.net/browse/JDK-8074531
For now, here are my changes again with Sean's suggested changes to the
package-info.java files.
http://cr.openjdk.java.net/~juh/8073430/01/
Thanks,
Jason
On 03/04/2015 07:14 PM, Wang Weijun wrote:
Hi Jason
I noticed several "@SuppressWarnings("deprecation")" in some sun.* or com.sun.*
classes and it makes me feel uncomfortable. The usage of this annotation, if I understand
correctly, means we know we should not use it but we have to use it because we are lazy or there
are no better alternatives. I highly doubt if any is the case here.
So, we should investigate how those methods are used. If they are strictly
internal (not jdk.exported) and not used inside JDK, remove them since they
will be inaccessible anymore in jdk9. If they are still used somewhere,
consider rewriting them (maybe in another fix). If they are jdk.exported,
rewrite if the deprecated interfaces is not shown in the API itself (type or
argument or return), otherwise, @deprecate them also.
Thanks
Max
On Mar 5, 2015, at 03:02, Jason Uh wrote:
webrev: http://cr.openjdk.java.net/~juh/8073430/00/
jbs: https://bugs.openjdk.java.net/browse/JDK-8073430
Please review this change, which deprecates the classes in java.security.acl
and javax.security.cert. These packages have been superseded by replacements
for a long time.
For java.security.acl, there have been replacement APIs available since JDK 1.2
in java.security.Policy and related classes. For javax.security.cert,
replacements have existed in java.security.cert since JDK 1.4. These
replacements have been noted in the javadocs, so applications using these old
APIs have had plenty of time to migrate.
Two methods
HandshakeCompletedEvent.getPeerCertificateChain
SSLSession.getPeerCertificateChain
that return the javax.security.cert.X509Certificate type will also be
deprecated.
The change also includes deprecation warning suppression in a few areas,
including sun.net.www.protocol.https.
Thanks,
Jason
Re: Code review of JDK-8072385, Only the first DNSName entry is checked for endpoint identification
webrev: http://cr.openjdk.java.net/~xuelei/8072385/webrev.00/ On 3/4/2015 10:51 PM, Xuelei Fan wrote: > Hi, > > Please review the fix for: >https://bugs.openjdk.java.net/browse/JDK-8072385 > > In SunJSSE implementation, during endpoint identification, there is a > bug in SubjectAlternativeName matching that only the fist DNSName are > checked. As may impact some business where host-name alias are used. > > The patch is attached. > > Thanks, > Xuelei >
Re: Review Request: 8074428, 8074429, 8074430 jdk.pack200, jdk.jartool, jdk.policytool modules
> On Mar 6, 2015, at 01:55, Mandy Chung wrote: > > For this review request, are you okay with this patch moving policytool and > jarsigner tools to the new home? Yes. --Max
Re: [9] RFR: 8073430: Deprecate security APIs that have been superseded
Looks good, Jason, just a couple of comments:
- Add an @Deprecated annotation to the package-info.java files since the
entire package is deprecated. Put {@code} around the package/class names.
- add a noreg label to the bug
--Sean
On 03/04/2015 02:02 PM, Jason Uh wrote:
webrev: http://cr.openjdk.java.net/~juh/8073430/00/
jbs: https://bugs.openjdk.java.net/browse/JDK-8073430
Please review this change, which deprecates the classes in
java.security.acl and javax.security.cert. These packages have been
superseded by replacements for a long time.
For java.security.acl, there have been replacement APIs available since
JDK 1.2 in java.security.Policy and related classes. For
javax.security.cert, replacements have existed in java.security.cert
since JDK 1.4. These replacements have been noted in the javadocs, so
applications using these old APIs have had plenty of time to migrate.
Two methods
HandshakeCompletedEvent.getPeerCertificateChain
SSLSession.getPeerCertificateChain
that return the javax.security.cert.X509Certificate type will also be
deprecated.
The change also includes deprecation warning suppression in a few areas,
including sun.net.www.protocol.https.
Thanks,
Jason
Re: Review Request: 8074428, 8074429, 8074430 jdk.pack200, jdk.jartool, jdk.policytool modules
Max, Since the new APIs you're working on are still in design phase, I think it's a bit early to discuss where these new APIs should be in. Just one thing to say about the new JarSigner API from your webrev. com.sun.jarsigner is an existing exported package that you should consider whether the new APIs should be added there rather than a new package. Anyway, we can discuss that when your work is further along. For this review request, are you okay with this patch moving policytool and jarsigner tools to the new home? Mandy [1] http://hg.openjdk.java.net/jdk9/dev/jdk/file/85b61f4eee66/src/jdk.dev/share/classes/com/sun/jarsigner/package-info.java On 3/5/15 1:13 AM, Weijun Wang wrote: There is no problem the new API be in a separate module. It is independent of keytool now. Said that, I've been thinking about rewriting keytool to use this new API. --Max On 3/5/2015 16:23, Alan Bateman wrote: On 05/03/2015 02:55, Wang Weijun wrote: - Move keytool to jdk.security.util, it's now in java.base but keytool is not part of Java SE spec (Of course, if that means keytool will be in JDK instead of JRE please stop. But will there will the name difference anymore? I am thinking of an installer like cygwin that you can choose anything except that java.base is checked grayed). The reason that keytool is in java.base is so that keys and certificates can be managed on a small runtime. It's the same reason that it is in our compact1 builds too. I wasn't aware of JDK-8058778 but it can't result in java.base exporting a JDK-specific API. -Alan
Re: Review Request: 8074428, 8074429, 8074430 jdk.pack200, jdk.jartool, jdk.policytool modules
On 3/5/2015 12:31 AM, Alan Bateman wrote: On 05/03/2015 01:13, Mandy Chung wrote: : Separate webrevs for each issue: 1. pack200, unpack200 to jdk.pack200 http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8074428%2b8074429%2b8074430/8074428/webrev.00/ I think this looks okay. In the unshuffle_list (for back/forward porting patches) then it lists specific files, I wonder if we can move that to directories. I also see remnants of the tracing API in that file, I assume they can be removed. I also notice the tracing API left in unshuffle_list should be removed. At some point the entire jdk.dev should go away and so I'm thinking to leave this cleanup for the removal of jdk.dev. Mandy
Re: Review Request: 8074428, 8074429, 8074430 jdk.pack200, jdk.jartool, jdk.policytool modules
On 05/03/2015 01:13, Mandy Chung wrote: : 2. jar, jarsigner to jdk.jartool http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8074428%2b8074429%2b8074430/8074429/webrev.00/ It seems reasonable to have both jar and jarsigner in the same module so I think this is good. This will also work if Max adds an API to the jarsigner tool as he mentioned in one of the mails. -Alan
Re: Review Request: 8074428, 8074429, 8074430 jdk.pack200, jdk.jartool, jdk.policytool modules
Hello Mandy, The build changes look ok to me. /Erik On 2015-03-05 02:13, Mandy Chung wrote: As listed in an open issue in JEP 200: The jdk.dev and jdk.runtime modules contain miscellaneous tools that do not obviously belong to any other module; these modules will eventually be either renamed or refactored. Currently there are jdk.javadoc, jdk.jconsole, jdk.jcmd modules in the JDK that are organized around its primary tool. Such organization is easy to name, document and understand. This patch proposes to move tools from jdk.runtime and jdk.dev to jdk.pack200, jdk.jartool, jdk.policytool modules. Overall Webrev that will be convenient to review the build change and modules.xml change. http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8074428%2b8074429%2b8074430/webrev.00/ Separate webrevs for each issue: 1. pack200, unpack200 to jdk.pack200 http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8074428%2b8074429%2b8074430/8074428/webrev.00/ 2. jar, jarsigner to jdk.jartool http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8074428%2b8074429%2b8074430/8074429/webrev.00/ 3. policytool to jdk.policytool http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8074428%2b8074429%2b8074430/8074430/webrev.00/ There are remaining tools in jdk.dev that will be handled separately. jdk.dev will disappear when all of the remaining tools find its home. Mandy
Re: Review Request: 8074428, 8074429, 8074430 jdk.pack200, jdk.jartool, jdk.policytool modules
There is no problem the new API be in a separate module. It is independent of keytool now. Said that, I've been thinking about rewriting keytool to use this new API. --Max On 3/5/2015 16:23, Alan Bateman wrote: On 05/03/2015 02:55, Wang Weijun wrote: - Move keytool to jdk.security.util, it's now in java.base but keytool is not part of Java SE spec (Of course, if that means keytool will be in JDK instead of JRE please stop. But will there will the name difference anymore? I am thinking of an installer like cygwin that you can choose anything except that java.base is checked grayed). The reason that keytool is in java.base is so that keys and certificates can be managed on a small runtime. It's the same reason that it is in our compact1 builds too. I wasn't aware of JDK-8058778 but it can't result in java.base exporting a JDK-specific API. -Alan
Re: Review Request: 8074428, 8074429, 8074430 jdk.pack200, jdk.jartool, jdk.policytool modules
On 5 Mar 2015, at 08:31, Alan Bateman wrote: > On 05/03/2015 01:13, Mandy Chung wrote: >> : >> >> Separate webrevs for each issue: >> 1. pack200, unpack200 to jdk.pack200 >> >> http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8074428%2b8074429%2b8074430/8074428/webrev.00/ >> > I think this looks okay. In the unshuffle_list (for back/forward porting > patches) then it lists specific files, I wonder if we can move that to > directories. It should be possible to reduced the following set down from: jdk/src/jdk.pack200/share/native/common-unpack/bands.cpp : jdk/src/share/native/com/sun/java/util/jar/pack/bands.cpp jdk/src/jdk.pack200/share/native/common-unpack/bands.h : jdk/src/share/native/com/sun/java/util/jar/pack/bands.h jdk/src/jdk.pack200/share/native/common-unpack/bytes.cpp : jdk/src/share/native/com/sun/java/util/jar/pack/bytes.cpp jdk/src/jdk.pack200/share/native/common-unpack/bytes.h : jdk/src/share/native/com/sun/java/util/jar/pack/bytes.h jdk/src/jdk.pack200/share/native/common-unpack/coding.cpp : jdk/src/share/native/com/sun/java/util/jar/pack/coding.cpp jdk/src/jdk.pack200/share/native/common-unpack/coding.h : jdk/src/share/native/com/sun/java/util/jar/pack/coding.h jdk/src/jdk.pack200/share/native/common-unpack/constants.h : jdk/src/share/native/com/sun/java/util/jar/pack/constants.h jdk/src/jdk.pack200/share/native/common-unpack/defines.h : jdk/src/share/native/com/sun/java/util/jar/pack/defines.h jdk/src/jdk.pack200/share/native/common-unpack/unpack.cpp : jdk/src/share/native/com/sun/java/util/jar/pack/unpack.cpp jdk/src/jdk.pack200/share/native/common-unpack/unpack.h : jdk/src/share/native/com/sun/java/util/jar/pack/unpack.h jdk/src/jdk.pack200/share/native/common-unpack/utils.cpp : jdk/src/share/native/com/sun/java/util/jar/pack/utils.cpp jdk/src/jdk.pack200/share/native/common-unpack/utils.h : jdk/src/share/native/com/sun/java/util/jar/pack/utils.h jdk/src/jdk.pack200/share/native/common-unpack/zip.cpp : jdk/src/share/native/com/sun/java/util/jar/pack/zip.cpp jdk/src/jdk.pack200/share/native/common-unpack/zip.h : jdk/src/share/native/com/sun/java/util/jar/pack/zip.h to jdk/src/jdk.pack200/share/native/common-unpack : jdk/src/share/native/com/sun/java/util/jar/pack ...since the script attempts to match the full path, before reducing. I did mean to do further reductions on this shuffle list, but it just doesn’t seem worth it now, and care needs to be taken to ensure both shuffling and unshuffling work correctly. -Chris. > I also see remnants of the tracing API in that file, I assume they can be > removed. > > -Alan.
Re: Review Request: 8074428, 8074429, 8074430 jdk.pack200, jdk.jartool, jdk.policytool modules
On 05/03/2015 01:13, Mandy Chung wrote: 3. policytool to jdk.policytool http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8074428%2b8074429%2b8074430/8074430/webrev.00/ This part looks good to me. -Alan
Re: Review Request: 8074428, 8074429, 8074430 jdk.pack200, jdk.jartool, jdk.policytool modules
On 05/03/2015 01:13, Mandy Chung wrote: : Separate webrevs for each issue: 1. pack200, unpack200 to jdk.pack200 http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8074428%2b8074429%2b8074430/8074428/webrev.00/ I think this looks okay. In the unshuffle_list (for back/forward porting patches) then it lists specific files, I wonder if we can move that to directories. I also see remnants of the tracing API in that file, I assume they can be removed. -Alan.
Re: Review Request: 8074428, 8074429, 8074430 jdk.pack200, jdk.jartool, jdk.policytool modules
On 05/03/2015 02:55, Wang Weijun wrote: - Move keytool to jdk.security.util, it's now in java.base but keytool is not part of Java SE spec (Of course, if that means keytool will be in JDK instead of JRE please stop. But will there will the name difference anymore? I am thinking of an installer like cygwin that you can choose anything except that java.base is checked grayed). The reason that keytool is in java.base is so that keys and certificates can be managed on a small runtime. It's the same reason that it is in our compact1 builds too. I wasn't aware of JDK-8058778 but it can't result in java.base exporting a JDK-specific API. -Alan
