You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2017/11/22 20:17:53 UTC

svn commit: r1816076 - in /tomcat/trunk: conf/ java/org/apache/catalina/authenticator/jaspic/ test/org/apache/catalina/authenticator/jaspic/ webapps/docs/

Author: markt
Date: Wed Nov 22 20:17:53 2017
New Revision: 1816076

URL: http://svn.apache.org/viewvc?rev=1816076&view=rev
Log:
Correctly handle the case when AuthConfigFactoryImpl.registerConfigProvider() is called with a provider name of <code>null</code>.
Patch provided by Lazar.
This closes #94

Modified:
    tomcat/trunk/conf/jaspic-providers.xsd
    tomcat/trunk/java/org/apache/catalina/authenticator/jaspic/AuthConfigFactoryImpl.java
    tomcat/trunk/java/org/apache/catalina/authenticator/jaspic/PersistentProviderRegistrations.java
    tomcat/trunk/test/org/apache/catalina/authenticator/jaspic/TestAuthConfigFactoryImpl.java
    tomcat/trunk/webapps/docs/changelog.xml

Modified: tomcat/trunk/conf/jaspic-providers.xsd
URL: http://svn.apache.org/viewvc/tomcat/trunk/conf/jaspic-providers.xsd?rev=1816076&r1=1816075&r2=1816076&view=diff
==============================================================================
--- tomcat/trunk/conf/jaspic-providers.xsd (original)
+++ tomcat/trunk/conf/jaspic-providers.xsd Wed Nov 22 20:17:53 2017
@@ -35,7 +35,7 @@
                 </xs:complexType>
               </xs:element>
             </xs:sequence>
-            <xs:attribute name="className" use="required" type="xs:string" />
+            <xs:attribute name="className" type="xs:string" />
             <xs:attribute name="layer" type="xs:string" />
             <xs:attribute name="appContext" type="xs:string" />
             <xs:attribute name="description" type="xs:string" />

Modified: tomcat/trunk/java/org/apache/catalina/authenticator/jaspic/AuthConfigFactoryImpl.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/authenticator/jaspic/AuthConfigFactoryImpl.java?rev=1816076&r1=1816075&r2=1816076&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/authenticator/jaspic/AuthConfigFactoryImpl.java (original)
+++ tomcat/trunk/java/org/apache/catalina/authenticator/jaspic/AuthConfigFactoryImpl.java Wed Nov 22 20:17:53 2017
@@ -105,6 +105,22 @@ public class AuthConfigFactoryImpl exten
             log.debug(sm.getString("authConfigFactoryImpl.registerClass",
                     className, layer, appContext));
         }
+
+        AuthConfigProvider provider = null;
+        if (className != null) {
+            provider = createAuthConfigProvider(className, properties);
+        }
+
+        String registrationID = getRegistrationID(layer, appContext);
+        RegistrationContextImpl registrationContextImpl = new RegistrationContextImpl(
+                layer, appContext, description, true, provider, properties);
+        addRegistrationContextImpl(layer, appContext, registrationID, registrationContextImpl);
+        return registrationID;
+    }
+
+
+    private AuthConfigProvider createAuthConfigProvider(String className,
+            @SuppressWarnings("rawtypes") Map properties) throws SecurityException {
         Class<?> clazz = null;
         AuthConfigProvider provider = null;
         try {
@@ -121,12 +137,7 @@ public class AuthConfigFactoryImpl exten
         } catch (ReflectiveOperationException | IllegalArgumentException e) {
             throw new SecurityException(e);
         }
-
-        String registrationID = getRegistrationID(layer, appContext);
-        RegistrationContextImpl registrationContextImpl = new RegistrationContextImpl(
-                layer, appContext, description, true, provider, properties);
-        addRegistrationContextImpl(layer, appContext, registrationID, registrationContextImpl);
-        return registrationID;
+        return provider;
     }
 
 
@@ -365,7 +376,9 @@ public class AuthConfigFactoryImpl exten
         if (registrationContextImpl != null && registrationContextImpl.isPersistent()) {
             Provider provider = new Provider();
             provider.setAppContext(registrationContextImpl.getAppContext());
-            provider.setClassName(registrationContextImpl.getProvider().getClass().getName());
+            if (registrationContextImpl.getProvider() != null) {
+                provider.setClassName(registrationContextImpl.getProvider().getClass().getName());
+            }
             provider.setDescription(registrationContextImpl.getDescription());
             provider.setLayer(registrationContextImpl.getMessageLayer());
             for (Entry<String,String> property : registrationContextImpl.getProperties().entrySet()) {

Modified: tomcat/trunk/java/org/apache/catalina/authenticator/jaspic/PersistentProviderRegistrations.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/authenticator/jaspic/PersistentProviderRegistrations.java?rev=1816076&r1=1816075&r2=1816076&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/authenticator/jaspic/PersistentProviderRegistrations.java (original)
+++ tomcat/trunk/java/org/apache/catalina/authenticator/jaspic/PersistentProviderRegistrations.java Wed Nov 22 20:17:53 2017
@@ -121,12 +121,12 @@ final class PersistentProviderRegistrati
                     "    xsi:schemaLocation=\"http://tomcat.apache.org/xml jaspic-providers.xsd\"\n" +
                     "    version=\"1.0\">\n");
             for (Provider provider : providers.providers) {
-                writer.write("  <provider className=\"");
-                writer.write(provider.getClassName());
+                writer.write("  <provider");
+                writeOptional("className", provider.getClassName(), writer);
                 writeOptional("layer", provider.getLayer(), writer);
                 writeOptional("appContext", provider.getAppContext(), writer);
                 writeOptional("description", provider.getDescription(), writer);
-                writer.write("\">\n");
+                writer.write(">\n");
                 for (Entry<String,String> entry : provider.getProperties().entrySet()) {
                     writer.write("    <property name=\"");
                     writer.write(entry.getKey());
@@ -169,8 +169,9 @@ final class PersistentProviderRegistrati
 
     private static void writeOptional(String name, String value, Writer writer) throws IOException {
         if (value != null) {
-            writer.write("\" " + name + "=\"");
+            writer.write(" " + name + "=\"");
             writer.write(value);
+            writer.write("\"");
         }
     }
 

Modified: tomcat/trunk/test/org/apache/catalina/authenticator/jaspic/TestAuthConfigFactoryImpl.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/authenticator/jaspic/TestAuthConfigFactoryImpl.java?rev=1816076&r1=1816075&r2=1816076&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/catalina/authenticator/jaspic/TestAuthConfigFactoryImpl.java (original)
+++ tomcat/trunk/test/org/apache/catalina/authenticator/jaspic/TestAuthConfigFactoryImpl.java Wed Nov 22 20:17:53 2017
@@ -27,13 +27,18 @@ import javax.security.auth.message.confi
 import javax.security.auth.message.config.AuthConfigProvider;
 import javax.security.auth.message.config.RegistrationListener;
 
+import org.junit.After;
 import org.junit.Assert;
+import org.junit.Before;
 import org.junit.Test;
 
 import org.apache.catalina.Globals;
 
 public class TestAuthConfigFactoryImpl {
 
+    private String oldCatalinaBase;
+    private static final File TEST_CONFIG_FILE = new File("test/conf/jaspic-providers.xml");
+
     @Test
     public void testRegistrationNullLayer() {
         doTestResistration(null,  "AC_1",  ":AC_1");
@@ -309,18 +314,34 @@ public class TestAuthConfigFactoryImpl {
     }
 
 
-    @Test
-    public void testRemovePersistentRegistration() {
+    @Before
+    public void setUp() {
         // set CATALINA_BASE to test so that the file with persistent providers will be written in test/conf folder
-        String oldCatalinaBase = System.getProperty(Globals.CATALINA_BASE_PROP);
+        oldCatalinaBase = System.getProperty(Globals.CATALINA_BASE_PROP);
         System.setProperty(Globals.CATALINA_BASE_PROP, "test");
 
-        File file = new File("test/conf/jaspic-providers.xml");
-        if (file.exists()) {
-            file.delete();
+        if (TEST_CONFIG_FILE.exists()) {
+            TEST_CONFIG_FILE.delete();
+        }
+    }
+
+
+    @After
+    public void cleanUp() {
+        if (oldCatalinaBase != null ) {
+            System.setProperty(Globals.CATALINA_BASE_PROP, oldCatalinaBase);
+        } else {
+            System.clearProperty(Globals.CATALINA_BASE_PROP);
+        }
+
+        if (TEST_CONFIG_FILE.exists()) {
+            TEST_CONFIG_FILE.delete();
         }
+    }
+
 
-        try {
+    @Test
+    public void testRemovePersistentRegistration() {
             AuthConfigFactory factory = new AuthConfigFactoryImpl();
             factory.registerConfigProvider(
                     SimpleAuthConfigProvider.class.getName(), null, "L_1", "AC_1", null);
@@ -334,17 +355,40 @@ public class TestAuthConfigFactoryImpl {
             for (String registrationId : registrationIds) {
                 Assert.assertNotEquals(registrationId2, registrationId);
             }
-        } finally {
-            if (oldCatalinaBase != null ) {
-                System.setProperty(Globals.CATALINA_BASE_PROP, oldCatalinaBase);
-            } else {
-                System.clearProperty(Globals.CATALINA_BASE_PROP);
-            }
+    }
+
 
-            if (file.exists()) {
-                file.delete();
+    @Test
+    public void testRegistrationNullClassName() {
+        doTestNullClassName(false, "L_1", "AC_1");
+    }
+
+
+    @Test
+    public void testRegistrationNullClassOverrideExisting() {
+        doTestNullClassName(true, "L_1", "AC_1");
+    }
+
+
+    @Test
+    public void testRegistrationNullClassNullLayerNullAppContext() {
+        doTestNullClassName(false, null, null);
+    }
+
+
+    private void doTestNullClassName(boolean shouldOverrideExistingProvider, String layer, String appContext) {
+            AuthConfigFactory factory = new AuthConfigFactoryImpl();
+            if (shouldOverrideExistingProvider) {
+                factory.registerConfigProvider(SimpleAuthConfigProvider.class.getName(), null, layer, appContext, null);
             }
-        }
+            String registrationId = factory.registerConfigProvider(null, null, layer, appContext, null);
+            factory.refresh();
+
+            String[] registrationIds = factory.getRegistrationIDs(null);
+            Set<String> ids = new HashSet<>(Arrays.asList(registrationIds));
+            Assert.assertTrue(ids.contains(registrationId));
+            AuthConfigProvider provider = factory.getConfigProvider(layer, appContext, null);
+            Assert.assertNull(provider);
     }
 
 

Modified: tomcat/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1816076&r1=1816075&r2=1816076&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Wed Nov 22 20:17:53 2017
@@ -122,6 +122,12 @@
         is persistent, it should be removed from the persistent store. Patch
         provided by Lazar. (markt)
       </fix>
+      <fix>
+        <bug>61784</bug>: Correctly handle the case when
+        <code>AuthConfigFactoryImpl.registerConfigProvider()</code> is called
+        with a provider name of <code>null</code>. Patch provided by Lazar.
+        (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Coyote">



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org