On 04/17/2014 07:38 AM, Wang Weijun wrote:
Hi All

There are two bugs. The first one is:

https://bugs.openjdk.java.net/browse/JDK-8040068 8040068:
SolarisSystem should be @Deprecated and @jdk.Exported(false)

of which the code change is simply

---
a/src/share/classes/com/sun/security/auth/module/SolarisSystem.java
+++
b/src/share/classes/com/sun/security/auth/module/SolarisSystem.java
@@ -29,8 +29,10 @@ * <p> This class implementation retrieves and
makes available Solaris * UID/GID/groups information for the current
user. * + * @deprecated replaced by {@link UnixSystem}. */
-@jdk.Exported +@jdk.Exported(false) +@Deprecated public class
SolarisSystem {

Looks fine.

The second one is:

https://bugs.openjdk.java.net/browse/JDK-8039951 8039951:
com.sun.security.auth.module missing classes on some platforms

of which the code change is at

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

This webrev contains two parts:

1. Changes to make/CompileJavaClasses.gmk to make sure the classes
are now created on all platforms, including the deprecated one in
8040068.

Looks fine.


2. Tweaks in the login modules to show a more friendly message.

Not sure if you like it, but the code change here has a side benefit
that allows the JAAS login config file below working on all platforms
(This is proved in the newly added regression test):

hello { com.sun.security.auth.module.UnixLoginModule optional;
com.sun.security.auth.module.NTLoginModule optional;
com.sun.security.auth.module.SolarisLoginModule optional; };

In fact, when a login module is not found, an exception will be
thrown immediately even if it's marked optional. Now that these
modules are available on all platforms, this won't happen anymore. If
you think this behavior is incorrect, we can fix it in another bug.

Is that because before your fix, it would throw NoClassDefFoundError?

I think the new behavior makes more sense and is definitely more user friendly. However, even though it is probably an obscure case, technically it is a change in behavior, so I think you should describe it in the CCC.

I noticed in a few classes, the previous code had a bug in it, ex:

143         if (ntSystem == null) {

which is not possible. So it's good you cleaned that up too.

--Sean

Reply via email to