On 08/06/2013 01:57 AM, Xuelei Fan wrote:
The fix looks fine.

BTW, I think the relationship between
javax.security.auth.login.Configuration and
javax.security.auth.login.ConfigurationSpi is not that instinctive in
the implementation.  For example, the impl of
Configuration.getAppConfigurationEntry() should be able to use the impl
of ConfigurationSpi.engineGetAppConfigurationEntry(). And the provider
should only need to extend ConfigurationSpi rather than the
Configuration class.  It would be nice to re-factoring the code if no
other concerns.

I'm not sure I understand the refactoring that you are suggesting, could you give an example? Also, ConfigurationSpi was added in JDK 1.6 after Configuration had already been added since 1.4. So we still need to preserve compatibility with implementations that extend from Configuration.

--Sean


Xuelei

On 8/2/2013 12:48 AM, Sean Mullan wrote:
Hello,

Could you please review my fix for 8016848:

webrev: http://cr.openjdk.java.net/~mullan/webrevs/8016848/webrev.00/

bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8016848

This is a bug because
javax.security.auth.login.Configuration.getConfiguration() throws a
ClassCastException when run with the compact1 or compact2 profiles.

This is because the default Configuration object that is returned is not
in the compact1 profile. The default Configuration is specified by the
"login.configuration.provider" security property (in the Java security
properties file). This property is currently set to
com.sun.security.auth.login.ConfigFile which is not in the compact1 or
compact2 profiles

The fix is to change the default value of the
"login.configuration.provider" security property to
sun.security.provider.ConfigFile, which will be a new class that is
essentially a copy of com.sun.security.auth.login.ConfigFile. However,
because it is in the sun.security.provider package, it will be in all
profiles. We will not remove the com.sun.security.auth.login.ConfigFile
class as there may be some applications directly using it.

noreg-jck as there is an existing JCK test for this.

Thanks,
Sean


Reply via email to