Author: robbie
Date: Thu Dec 20 16:06:30 2012
New Revision: 1424556
URL: http://svn.apache.org/viewvc?rev=1424556&view=rev
Log:
QPID-4513: improve client handling of discovery that its SASL Provider has
already been registered when it attemts to do so, as occurs in cases with
multiple classloaders.
Verifies if the previously registered Provider matches the new Provider trying
to be registerered; accepts it if it does, logs a warning if it doesnt (and
logs the properties at debug to aid discovering why), and now only logs the
error if we cant determine either way (rather than all the time as it did
previously). Also corrects and clarifies some of the other existing logging to
make it clearer.
Work by Alex (orudyy) and myself.
Added:
qpid/trunk/qpid/java/client/src/test/java/org/apache/qpid/client/security/DynamicSaslRegistrarTest.java
Modified:
qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/security/DynamicSaslRegistrar.java
qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/security/JCAProvider.java
Modified:
qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/security/DynamicSaslRegistrar.java
URL:
http://svn.apache.org/viewvc/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/security/DynamicSaslRegistrar.java?rev=1424556&r1=1424555&r2=1424556&view=diff
==============================================================================
---
qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/security/DynamicSaslRegistrar.java
(original)
+++
qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/security/DynamicSaslRegistrar.java
Thu Dec 20 16:06:30 2012
@@ -28,8 +28,10 @@ import org.apache.qpid.util.FileUtils;
import javax.security.sasl.SaslClientFactory;
import java.io.IOException;
import java.io.InputStream;
+import java.security.Provider;
import java.security.Security;
import java.util.Enumeration;
+import java.util.HashMap;
import java.util.Map;
import java.util.Properties;
import java.util.TreeMap;
@@ -67,10 +69,10 @@ public class DynamicSaslRegistrar
}
/** Reads the properties file, and creates a dynamic security provider to
register the SASL implementations with. */
- public static void registerSaslProviders()
+ public static ProviderRegistrationResult registerSaslProviders()
{
_logger.debug("public static void registerSaslProviders(): called");
-
+ ProviderRegistrationResult result = ProviderRegistrationResult.FAILED;
// Open the SASL properties file, using the default name is one is not
specified.
String filename = System.getProperty(FILE_PROPERTY);
InputStream is =
@@ -89,22 +91,45 @@ public class DynamicSaslRegistrar
if (factories.size() > 0)
{
// Ensure we are used before the defaults
- if (Security.insertProviderAt(new JCAProvider(factories), 1)
== -1)
+ JCAProvider qpidProvider = new JCAProvider(factories);
+ if (Security.insertProviderAt(qpidProvider, 1) == -1)
{
- _logger.error("Unable to load custom SASL providers.");
+ Provider registeredProvider =
findProvider(JCAProvider.QPID_CLIENT_SASL_PROVIDER_NAME);
+ if (registeredProvider == null)
+ {
+ result = ProviderRegistrationResult.FAILED;
+ _logger.error("Unable to load custom SASL providers.");
+ }
+ else if (registeredProvider.equals(qpidProvider))
+ {
+ result =
ProviderRegistrationResult.EQUAL_ALREADY_REGISTERED;
+ _logger.debug("Custom SASL provider is already
registered with equal properties.");
+ }
+ else
+ {
+ result =
ProviderRegistrationResult.DIFFERENT_ALREADY_REGISTERED;
+ _logger.warn("Custom SASL provider was already
registered with different properties.");
+ if (_logger.isDebugEnabled())
+ {
+ _logger.debug("Custom SASL provider " +
registeredProvider + " properties: " + new HashMap<Object,
Object>(registeredProvider));
+ }
+ }
}
else
{
+ result = ProviderRegistrationResult.SUCCEEDED;
_logger.info("Additional SASL providers successfully
registered.");
}
}
else
{
- _logger.warn("No additional SASL providers registered.");
+ result = ProviderRegistrationResult.NO_SASL_FACTORIES;
+ _logger.warn("No additional SASL factories found to
register.");
}
}
catch (IOException e)
{
+ result = ProviderRegistrationResult.FAILED;
_logger.error("Error reading properties: " + e, e);
}
finally
@@ -122,6 +147,22 @@ public class DynamicSaslRegistrar
}
}
}
+ return result;
+ }
+
+ static Provider findProvider(String name)
+ {
+ Provider[] providers = Security.getProviders();
+ Provider registeredProvider = null;
+ for (Provider provider : providers)
+ {
+ if (name.equals(provider.getName()))
+ {
+ registeredProvider = provider;
+ break;
+ }
+ }
+ return registeredProvider;
}
/**
@@ -158,15 +199,24 @@ public class DynamicSaslRegistrar
continue;
}
- _logger.debug("Registering class "+ clazz.getName() +" for
mechanism "+mechanism);
+ _logger.debug("Found class "+ clazz.getName() +" for mechanism
"+mechanism);
factoriesToRegister.put(mechanism, (Class<? extends
SaslClientFactory>) clazz);
}
catch (Exception ex)
{
- _logger.error("Error instantiating SaslClientFactory calss " +
className + " - skipping");
+ _logger.error("Error instantiating SaslClientFactory class " +
className + " - skipping");
}
}
return factoriesToRegister;
}
+
+ public static enum ProviderRegistrationResult
+ {
+ SUCCEEDED,
+ EQUAL_ALREADY_REGISTERED,
+ DIFFERENT_ALREADY_REGISTERED,
+ NO_SASL_FACTORIES,
+ FAILED;
+ }
}
Modified:
qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/security/JCAProvider.java
URL:
http://svn.apache.org/viewvc/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/security/JCAProvider.java?rev=1424556&r1=1424555&r2=1424556&view=diff
==============================================================================
---
qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/security/JCAProvider.java
(original)
+++
qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/security/JCAProvider.java
Thu Dec 20 16:06:30 2012
@@ -39,6 +39,11 @@ import java.util.Map;
*/
public class JCAProvider extends Provider
{
+ static final String QPID_CLIENT_SASL_PROVIDER_NAME =
"AMQSASLProvider-Client";
+ static final String QPID_CLIENT_SASL_PROVIDER_INFO = "A JCA provider that
registers all "
+ + "AMQ SASL providers
that want to be registered";
+ static final double QPID_CLIENT_SASL_PROVIDER_VERSION = 1.0;
+
private static final Logger log =
LoggerFactory.getLogger(JCAProvider.class);
/**
@@ -48,8 +53,7 @@ public class JCAProvider extends Provide
*/
public JCAProvider(Map<String, Class<? extends SaslClientFactory>>
providerMap)
{
- super("AMQSASLProvider-Client", 1.0, "A JCA provider that registers
all "
- + "AMQ SASL providers that want to be registered");
+ super(QPID_CLIENT_SASL_PROVIDER_NAME,
QPID_CLIENT_SASL_PROVIDER_VERSION, QPID_CLIENT_SASL_PROVIDER_INFO);
register(providerMap);
}
@@ -63,7 +67,7 @@ public class JCAProvider extends Provide
for (Map.Entry<String, Class<? extends SaslClientFactory>> me :
providerMap.entrySet())
{
put( "SaslClientFactory."+me.getKey(), me.getValue().getName());
- log.debug("Registered SASL Client factory for " + me.getKey() + "
as " + me.getValue().getName());
+ log.debug("Recording SASL Client factory for " + me.getKey() + "
as " + me.getValue().getName());
}
}
}
Added:
qpid/trunk/qpid/java/client/src/test/java/org/apache/qpid/client/security/DynamicSaslRegistrarTest.java
URL:
http://svn.apache.org/viewvc/qpid/trunk/qpid/java/client/src/test/java/org/apache/qpid/client/security/DynamicSaslRegistrarTest.java?rev=1424556&view=auto
==============================================================================
---
qpid/trunk/qpid/java/client/src/test/java/org/apache/qpid/client/security/DynamicSaslRegistrarTest.java
(added)
+++
qpid/trunk/qpid/java/client/src/test/java/org/apache/qpid/client/security/DynamicSaslRegistrarTest.java
Thu Dec 20 16:06:30 2012
@@ -0,0 +1,140 @@
+/*
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ */
+package org.apache.qpid.client.security;
+
+import java.io.File;
+import java.security.Provider;
+import java.security.Security;
+
+import
org.apache.qpid.client.security.DynamicSaslRegistrar.ProviderRegistrationResult;
+import org.apache.qpid.test.utils.QpidTestCase;
+import org.apache.qpid.test.utils.TestFileUtils;
+
+public class DynamicSaslRegistrarTest extends QpidTestCase
+{
+ private Provider _registeredProvider;
+
+ public void setUp() throws Exception
+ {
+ super.setUp();
+
+ //If the client provider is already registered, remove it for the
duration of the test
+ _registeredProvider =
DynamicSaslRegistrar.findProvider(JCAProvider.QPID_CLIENT_SASL_PROVIDER_NAME);
+ if (_registeredProvider != null)
+ {
+
Security.removeProvider(JCAProvider.QPID_CLIENT_SASL_PROVIDER_NAME);
+ }
+ }
+
+ public void tearDown() throws Exception
+ {
+ //Remove any provider left behind by the test.
+ Security.removeProvider(JCAProvider.QPID_CLIENT_SASL_PROVIDER_NAME);
+ try
+ {
+ //If the client provider was already registered before the test,
restore it.
+ if (_registeredProvider != null)
+ {
+ Security.insertProviderAt(_registeredProvider, 1);
+ }
+ }
+ finally
+ {
+ super.tearDown();
+ }
+ }
+
+ public void testRegisterDefaultProvider()
+ {
+ assertNull("Provider should not yet be registered",
DynamicSaslRegistrar.findProvider(JCAProvider.QPID_CLIENT_SASL_PROVIDER_NAME));
+
+ ProviderRegistrationResult firstRegistrationResult =
DynamicSaslRegistrar.registerSaslProviders();
+ assertEquals("Unexpected registration result",
ProviderRegistrationResult.SUCCEEDED, firstRegistrationResult);
+ assertNotNull("Providers should now be registered",
DynamicSaslRegistrar.findProvider(JCAProvider.QPID_CLIENT_SASL_PROVIDER_NAME));
+ }
+
+ public void testRegisterDefaultProviderTwice()
+ {
+ assertNull("Provider should not yet be registered",
DynamicSaslRegistrar.findProvider(JCAProvider.QPID_CLIENT_SASL_PROVIDER_NAME));
+
+ DynamicSaslRegistrar.registerSaslProviders();
+ assertNotNull("Providers should now be registered",
DynamicSaslRegistrar.findProvider(JCAProvider.QPID_CLIENT_SASL_PROVIDER_NAME));
+
+ ProviderRegistrationResult result =
DynamicSaslRegistrar.registerSaslProviders();
+ assertEquals("Unexpected registration result when trying to
re-register", ProviderRegistrationResult.EQUAL_ALREADY_REGISTERED, result);
+ assertNotNull("Providers should still be registered",
DynamicSaslRegistrar.findProvider(JCAProvider.QPID_CLIENT_SASL_PROVIDER_NAME));
+ }
+
+ @SuppressWarnings("serial")
+ public void
testRegisterDefaultProviderWhenAnotherIsAlreadyPresentWithDifferentFactories()
+ {
+ assertNull("Provider should not be registered",
DynamicSaslRegistrar.findProvider(JCAProvider.QPID_CLIENT_SASL_PROVIDER_NAME));
+
+ //Add a test provider with the same name, version, info as the default
client provider, but with different factory properties (none).
+ Provider testProvider = new
Provider(JCAProvider.QPID_CLIENT_SASL_PROVIDER_NAME,
+
JCAProvider.QPID_CLIENT_SASL_PROVIDER_VERSION,
+
JCAProvider.QPID_CLIENT_SASL_PROVIDER_INFO){};
+ Security.addProvider(testProvider);
+ assertSame("Test provider should be registered", testProvider,
DynamicSaslRegistrar.findProvider(JCAProvider.QPID_CLIENT_SASL_PROVIDER_NAME));
+
+ //Try to register the default provider now that another with the same
name etc (but different factories)
+ //is already registered, expect it not to be registered as a result.
+ ProviderRegistrationResult result =
DynamicSaslRegistrar.registerSaslProviders();
+ assertEquals("Unexpected registration result",
ProviderRegistrationResult.DIFFERENT_ALREADY_REGISTERED, result);
+
+ //Verify the test provider is still registered
+ assertSame("Test provider should still be registered", testProvider,
DynamicSaslRegistrar.findProvider(JCAProvider.QPID_CLIENT_SASL_PROVIDER_NAME));
+ }
+
+ public void testRegisterWithNoFactories()
+ {
+ File emptyTempFile = TestFileUtils.createTempFile(this);
+
+ assertNull("Provider should not be registered",
DynamicSaslRegistrar.findProvider(JCAProvider.QPID_CLIENT_SASL_PROVIDER_NAME));
+
+ //Adjust the location of the properties file to point at an empty
file, so no factories are found to register.
+ setTestSystemProperty("amq.dynamicsaslregistrar.properties",
emptyTempFile.getPath());
+
+ //Try to register the default provider, expect it it not to be
registered because there were no factories.
+ ProviderRegistrationResult result =
DynamicSaslRegistrar.registerSaslProviders();
+ assertEquals("Unexpected registration result",
ProviderRegistrationResult.NO_SASL_FACTORIES, result);
+
+ assertNull("Provider should not be registered",
DynamicSaslRegistrar.findProvider(JCAProvider.QPID_CLIENT_SASL_PROVIDER_NAME));
+ }
+
+ public void testRegisterWithMissingFileGetsDefault()
+ {
+ //Create a temp file and then delete it, such that we get a path which
doesn't exist
+ File tempFile = TestFileUtils.createTempFile(this);
+ assertTrue("Failed to delete file", tempFile.delete());
+
+ assertNull("Provider should not be registered",
DynamicSaslRegistrar.findProvider(JCAProvider.QPID_CLIENT_SASL_PROVIDER_NAME));
+
+ //Adjust the location of the properties file to point at non-existent
file.
+ setTestSystemProperty("amq.dynamicsaslregistrar.properties",
tempFile.getPath());
+
+ //Try to register the default provider, expect it to fall back to the
default in the jar and succeed.
+ ProviderRegistrationResult result =
DynamicSaslRegistrar.registerSaslProviders();
+ assertEquals("Unexpected registration result",
ProviderRegistrationResult.SUCCEEDED, result);
+
+ assertNotNull("Provider should be registered",
DynamicSaslRegistrar.findProvider(JCAProvider.QPID_CLIENT_SASL_PROVIDER_NAME));
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
