Merge in the allow-flawed branch, as there were no objections.

git-svn-id: https://svn.apache.org/repos/asf/jakarta/commons/proper/logging/trunk@190565 13f79535-47bb-0310-9956-ffa450edef68

src/java/org/apache/commons/logging/impl/LogFactoryImpl.java

@@ -20,8 +20,6 @@ package org.apache.commons.logging.impl;
 import java.lang.reflect.Constructor;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
-import java.security.AccessController;
-import java.security.PrivilegedAction;
 import java.util.Enumeration;
 import java.util.Hashtable;
 import java.util.Vector;
@@ -99,6 +97,49 @@ public class LogFactoryImpl extends LogFactory {
     protected static final String LOG_PROPERTY_OLD =
         "org.apache.commons.logging.log";
 
+    /**
+     * The name of the system property which can be set true/false to
+     * determine system behaviour when a bad context-classloader is encountered.
+     * When set to false
+     * 
+     * Default behaviour: true (tolerates bad context classloaders)
+     * 
+     * See also method setAttribute.
+     */
+    public static final String ALLOW_FLAWED_CONTEXT_PROPERTY = 
+        "org.apache.commons.logging.Log.allowFlawedContext";
+
+    /**
+     * The name of the system property which can be set true/false to
+     * determine system behaviour when a bad logging adapter class is
+     * encountered during logging discovery. When set to false, an
+     * exception will be thrown and the app will fail to start. When set
+     * to true, discovery will continue (though the user might end up
+     * with a different logging implementation than they expected).
+     * 
+     * Default behaviour: true (tolerates bad logging adapters)
+     * 
+     * See also method setAttribute.
+     */
+    public static final String ALLOW_FLAWED_DISCOVERY_PROPERTY = 
+        "org.apache.commons.logging.Log.allowFlawedDiscovery";
+
+    /**
+     * The name of the system property which can be set true/false to
+     * determine system behaviour when a logging adapter class is
+     * encountered which has bound to the wrong Log class implementation.
+     * When set to false, an exception will be thrown and the app will fail
+     * to start. When set to true, discovery will continue (though the user
+     * might end up with a different logging implementation than they expected).
+     * 
+     * Default behaviour: true (tolerates bad Log class hierarchy)
+     * 
+     * See also method setAttribute.
+     */
+    public static final String ALLOW_FLAWED_HIERARCHY_PROPERTY = 
+        "org.apache.commons.logging.Log.allowFlawedHierarchy";
+
+
 
     // ----------------------------------------------------- Instance Variables
 
@@ -158,6 +199,21 @@ public class LogFactoryImpl extends LogFactory {
     protected Class logMethodSignature[] =
     { LogFactory.class };
 
+    /**
+     * See getBaseClassLoader and initConfiguration.
+     */
+    private boolean allowFlawedContext;
+    
+    /**
+     * See handleFlawedDiscovery and initConfiguration.
+     */
+    private boolean allowFlawedDiscovery;
+    
+    /**
+     * See handleFlawedHierarchy and initConfiguration.
+     */
+    private boolean allowFlawedHierarchy;
+    
     // --------------------------------------------------------- Public Methods
 
 
@@ -272,6 +328,21 @@ public class LogFactoryImpl extends LogFactory {
      * Set the configuration attribute with the specified name.  Calling
      * this with a <code>null</code> value is equivalent to calling
      * <code>removeAttribute(name)</code>.
+     * <p>
+     * This method can be used to set logging configuration programmatically
+     * rather than via system properties. It can also be used in code running
+     * within a container (such as a webapp) to configure behaviour on a
+     * per-component level instead of globally as system properties would do.
+     * To use this method instead of a system property, call
+     * <pre>
+     * LogFactory.getFactory().setAttribute(...)
+     * </pre>
+     * This must be done before the first Log object is created; configuration
+     * changes after that point will be ignored.
+     * <p>
+     * This method is also called automatically if LogFactory detects a
+     * commons-logging.properties file; every entry in that file is set
+     * automatically as an attribute here.
      *
      * @param name Name of the attribute to set
      * @param value Value of the attribute to set, or <code>null</code>
@@ -279,6 +350,10 @@ public class LogFactoryImpl extends LogFactory {
      */
     public void setAttribute(String name, Object value) {
 
+        if (logConstructor != null) {
+            logDiagnostic("setAttribute: call too late; configuration already performed.");
+        }
+
         if (value == null) {
             attributes.remove(name);
         } else {
@@ -406,54 +481,6 @@ public class LogFactoryImpl extends LogFactory {
     }
     
 
-    /**
-     * MUST KEEP THIS METHOD PRIVATE.
-     *
-     * <p>Exposing this method outside of
-     * <code>org.apache.commons.logging.LogFactoryImpl</code>
-     * will create a security violation:
-     * This method uses <code>AccessController.doPrivileged()</code>.
-     * </p>
-     *
-     * Load a class, try first the thread class loader, and
-     * if it fails use the loader that loaded this class.
-     * 
-     * @param name fully qualified class name of the class to load
-     *                          
-     * @throws LinkageError if the linkage fails
-     * @throws ExceptionInInitializerError if the initialization provoked
-     *            by this method fails
-     * @throws ClassNotFoundException if the class cannot be located
-     */
-    private static Class loadClass( final String name )
-        throws ClassNotFoundException
-    {
-        Object result = AccessController.doPrivileged(
-            new PrivilegedAction() {
-                public Object run() {
-                    ClassLoader threadCL = getContextClassLoader();
-                    if (threadCL != null) {
-                        try {
-                            return threadCL.loadClass(name);
-                        } catch( ClassNotFoundException ex ) {
-                            // ignore
-                        }
-                    }
-                    try {
-                        return Class.forName( name );
-                    } catch (ClassNotFoundException e) {
-                        return e;
-                    }
-                }
-            });
-
-        if (result instanceof Class)
-            return (Class)result;
-        
-        throw (ClassNotFoundException)result;
-    }
-
-
     /**
      * Is <em>JDK 1.3 with Lumberjack</em> logging available?   
      * 
@@ -575,6 +602,63 @@ public class LogFactoryImpl extends LogFactory {
             return false;
         }
     }
+
+    /**
+     * Attempt to find an attribute (see method setAttribute) or a 
+     * system property with the provided name and return its value.
+     * <p>
+     * The attributes associated with this object are checked before
+     * system properties in case someone has explicitly called setAttribute,
+     * or a configuration property has been set in a commons-logging.properties
+     * file.
+     * 
+     * @return the value associated with the property, or null.
+     */
+    private String getConfigurationValue(String property) {
+        logDiagnostic("Trying to get configuration for item " + property);
+    
+        logDiagnostic("Looking for attribute " + property);
+        Object valueObj =  getAttribute(property);
+        if (valueObj != null) {
+            logDiagnostic("Found value [" + valueObj + "] for " + property);
+            return valueObj.toString();
+        }
+        
+        logDiagnostic("Looking for system property " + property);
+        try {
+            String value = System.getProperty(LOG_PROPERTY);
+            logDiagnostic("Found value [" + value + "] for " + property);
+            return value;
+        } catch (SecurityException e) {
+            logDiagnostic("Security prevented reading system property.");
+        }
+        
+        return null;
+    }
+    
+    /**
+     * Get the setting for the user-configurable behaviour specified by key.
+     * If nothing has explicitly been set, then return dflt.  
+     */
+    private boolean getBooleanConfiguration(String key, boolean dflt) {
+        String val = getConfigurationValue(key);
+        if (val == null)
+            return dflt;
+        return Boolean.valueOf(val).booleanValue();
+    }
+
+    /**
+     * Initialize a number of variables that control the behaviour of this
+     * class and that can be tweaked by the user. This is done when the first
+     * logger is created, not in the constructor of this class, because we
+     * need to give the user a chance to call method setAttribute in order to
+     * configure this object.
+     */
+    private void initConfiguration() {
+        allowFlawedContext = getBooleanConfiguration(ALLOW_FLAWED_CONTEXT_PROPERTY, true);
+        allowFlawedDiscovery = getBooleanConfiguration(ALLOW_FLAWED_DISCOVERY_PROPERTY, true);
+        allowFlawedHierarchy = getBooleanConfiguration(ALLOW_FLAWED_HIERARCHY_PROPERTY, true);
+    }
   
 
     /**
@@ -591,6 +675,8 @@ public class LogFactoryImpl extends LogFactory {
     {
         logDiagnostic("Attempting to discover a Log implementation.");
         
+        initConfiguration();
+        
         Log result = null;
         
         // See if the user specified the Log implementation to use
@@ -713,58 +799,101 @@ public class LogFactoryImpl extends LogFactory {
 
         logDiagnostic("Attempting to instantiate " + logAdapterClassName);
         
-        Class logAdapterClass = null;
-        
         Object[] params = { logCategory };
         Log logAdapter = null;
         Constructor constructor = null;
-        try {
+        
-            logAdapterClass = loadClass(logAdapterClassName);
+        Class logAdapterClass = null;
-            constructor = logAdapterClass.getConstructor(logConstructorSignature);
+        ClassLoader currentCL = getBaseClassLoader();
-            logAdapter = (Log) constructor.newInstance(params);
+        
-        } catch (ClassNotFoundException e) {
+        for(;;) {
-            // We were unable to find the log adapter
+            try {
-            String msg = "" + e.getMessage();
+                Class c = Class.forName(logAdapterClassName, true, currentCL);
-            logDiagnostic(
+                constructor = c.getConstructor(logConstructorSignature);
+                Object o = constructor.newInstance(params);
+                
+                // Note that we do this test after trying to create an instance
+                // [rather than testing Log.class.isAssignableFrom(c)] so that
+                // we don't complain about Log hierarchy problems when the
+                // adapter couldn't be instantiated anyway.
+                if (o instanceof Log) {
+                    logAdapterClass = c;
+                    logAdapter = (Log) o;
+                    break;
+                }
+            
+                
+                // Oops, we have a potential problem here. An adapter class
+                // has been found and its underlying lib is present too, but
+                // there are multiple Log interface classes available making it
+                // impossible to cast to the type the caller wanted. We 
+                // certainly can't use this logger, but we need to know whether
+                // to keep on discovering or terminate now.
+                //
+                // The handleFlawedHierarchy method will throw 
+                // LogConfigurationException if it regards this problem as
+                // fatal, and just return if not.
+                handleFlawedHierarchy(currentCL, c);
+            } catch (ClassNotFoundException e) {
+                // The current classloader was unable to find the log adapter 
+                // in this or any ancestor classloader. There's no point in
+                // trying higher up in the hierarchy in this case..
+                String msg = "" + e.getMessage();
+                logDiagnostic(
                     "The log adapter "
                     + logAdapterClassName
-                    + " is not available: " 
+                    + " is not available via classloader " 
+                    + objectId(currentCL)
+                    + ": "
                     + msg.trim());
-            return null;
+                break;
-         } catch (NoClassDefFoundError e) {
+            } catch (NoClassDefFoundError e) {
-            // We were able to load the adapter but it had references to
+                // We were able to load the adapter but it had references to
-            // other classes that could not be found. This simply means that
+                // other classes that could not be found. This simply means that
-            // the underlying logger library could not be found.
+                // the underlying logger library is not present in this or any
-            String msg = "" + e.getMessage();
+                // ancestor classloader. There's no point in trying higher up
-            logDiagnostic(
+                // in the hierarchy in this case..
-                    "The logging library used by "
+                String msg = "" + e.getMessage();
+                logDiagnostic(
+                    "The log adapter "
                     + logAdapterClassName
-                    + " is not available: " 
+                    + " is missing dependencies when loaded via classloader "
+                    + objectId(currentCL)
+                    + ": "
                     + msg.trim());
-            return null;
+                break;
-        } catch (ExceptionInInitializerError e) {
+            } catch (ExceptionInInitializerError e) {
-            // A static initializer block or the initializer code associated 
+                // A static initializer block or the initializer code associated 
-            // with a static variable on the log adapter class has thrown
+                // with a static variable on the log adapter class has thrown
-            // an exception.
+                // an exception.
-            //
+                //
-            // We treat this as meaning the adapter's underlying logging
+                // We treat this as meaning the adapter's underlying logging
-            // library could not be found.
+                // library could not be found.
-            String msg = "" + e.getMessage();
+                String msg = "" + e.getMessage();
-            logDiagnostic(
+                logDiagnostic(
-                    "The logging library used by "
+                    "The log adapter "
                     + logAdapterClassName
-                    + " is not available: "
+                    + " is unable to initialize itself when loaded via classloader "
+                    + objectId(currentCL)
+                    + ": "
                     + msg.trim());
-            return null;
+                break;
-        } catch(Throwable t) {
+            } catch(Throwable t) {
-            // handleFlawedDiscovery will determine whether this is a fatal
+                // handleFlawedDiscovery will determine whether this is a fatal
-            // problem or not. If it is fatal, then a LogConfigurationException
+                // problem or not. If it is fatal, then a LogConfigurationException
-            // will be thrown.
+                // will be thrown.
-            handleFlawedDiscovery(logAdapterClassName, logAdapterClass, t);
+                handleFlawedDiscovery(logAdapterClassName, currentCL, t);
-            return null;
+            }
+                        
+            if (currentCL == null) {
+                break;
+            }
+            
+            // try the parent classloader
+            currentCL = currentCL.getParent();
         }
-        
+
-        if (affectState) {
+        if ((logAdapter != null) && affectState) {
             // We've succeeded, so set instance fields
             this.logClassName   = logAdapterClassName;
             this.logConstructor = constructor;
@@ -786,58 +915,136 @@ public class LogFactoryImpl extends LogFactory {
     }
     
     
+    /**
+     * Return the classloader from which we should try to load the logging
+     * adapter classes.
+     * <p>
+     * This method usually returns the context classloader. However if it
+     * is discovered that the classloader which loaded this class is a child
+     * of the context classloader <i>and</i> the allowFlawedContext option
+     * has been set then the classloader which loaded this class is returned
+     * instead.
+     * <p>
+     * The only time when the classloader which loaded this class is a
+     * descendant (rather than the same as or an ancestor of the context
+     * classloader) is when an app has created custom classloaders but
+     * failed to correctly set the context classloader. This is a bug in
+     * the calling application; however we provide the option for JCL to
+     * simply generate a warning rather than fail outright.
+     * 
+     */
+    private ClassLoader getBaseClassLoader() throws LogConfigurationException {
+        ClassLoader contextClassLoader = getContextClassLoader();
+        ClassLoader thisClassLoader = getClassLoader(LogFactoryImpl.class);
+        
+        ClassLoader baseClassLoader = getLowestClassLoader(
+                contextClassLoader, thisClassLoader);
+
+        if (baseClassLoader == null) {
+            throw new LogConfigurationException(
+                "Bad classloader hierarchy; LogFactoryImpl was loaded via"
+                + " a classloader that is not related to the current context"
+                + " classloader."); 
+        }
+
+        if (baseClassLoader != contextClassLoader) {
+            // We really should just use the contextClassLoader as the starting
+            // point for scanning for log adapter classes. However it is expected
+            // that there are a number of broken systems out there which create
+            // custom classloaders but fail to set the context classloader so
+            // we handle those flawed systems anyway.
+            if (allowFlawedContext) {
+                logDiagnostic(
+                "Warning: the context classloader is an ancestor of the"
+                + " classloader that loaded LogFactoryImpl; it should be"
+                + " the same or a descendant. The application using"
+                + " commons-logging should ensure the context classloader"
+                + " is used correctly.");
+            } else {
+                throw new LogConfigurationException(
+                        "Bad classloader hierarchy; LogFactoryImpl was loaded via"
+                        + " a classloader that is not related to the current context"
+                        + " classloader."); 
+            }
+        }
+        
+        return baseClassLoader;
+    }
+
+    /**
+     * Given two related classloaders, return the one which is a child of
+     * the other.
+     * <p>
+     * @param c1 is a classloader (including the null classloader)
+     * @param c2 is a classloader (including the null classloader)
+     * 
+     * @return c1 if it has c2 as an ancestor, c2 if it has c1 as an ancestor,
+     * and null if neither is an ancestor of the other.
+     */
+    private ClassLoader getLowestClassLoader(ClassLoader c1, ClassLoader c2) {
+        // TODO: use AccessController when dealing with classloaders here
+        
+        if (c1 == null)
+            return c2;
+        
+        if (c2 == null)
+            return c1;
+        
+        ClassLoader current;
+
+        // scan c1's ancestors to find c2
+        current = c1;
+        while (current != null) {
+            if (current == c2)
+                return c1;
+            current = current.getParent();
+        }
+       
+       // scan c2's ancestors to find c1
+        // scan c1's ancestors to find c2
+        current = c2;
+        while (current != null) {
+            if (current == c1)
+                return c2;
+            current = current.getParent();
+        }
+
+        return null;
+    }
+
     /**
      * Generates an internal diagnostic logging of the discovery failure and 
      * then throws a <code>LogConfigurationException</code> that wraps 
      * the passed <code>Throwable</code>.
      * 
-     * @param logAdapterClassName  the class name of the Log implementation
+     * @param logAdapterClassName is the class name of the Log implementation
      * that could not be instantiated. Cannot be <code>null</code>.
      * 
-     * @param logAdapterClass  <code>Code</code> whose name is
+     * @param classLoader is the classloader that we were trying to load the
-     * <code>logClassName</code>, or <code>null</code> if discovery was unable
+     * logAdapterClassName from when the exception occurred.
-     * to load the class.
      * 
-     * @param discoveryFlaw Throwable thrown during discovery.
+     * @param discoveryFlaw is the Throwable created by the classloader
      * 
      * @throws LogConfigurationException    ALWAYS
      */
     private void handleFlawedDiscovery(String logAdapterClassName,
-                                       Class logAdapterClass,
+                                       ClassLoader classLoader,
                                        Throwable discoveryFlaw) {
         
-        // Output diagnostics
+        logDiagnostic("Could not instantiate Log "
-        
+                      + logAdapterClassName + " -- "
-        // For ClassCastException use the more complex diagnostic
+                      + discoveryFlaw.getLocalizedMessage());        
-        // that analyzes the classloader hierarchy
+
-        if (discoveryFlaw instanceof ClassCastException
+        if (!allowFlawedDiscovery) {
-            && logAdapterClass != null) {
-            // reportInvalidAdapter returns a LogConfigurationException
-            // that wraps the ClassCastException; replace variable 
-            // 'discoveryFlaw' with that so we can rethrow the LCE
-            discoveryFlaw = reportInvalidLogAdapter(logAdapterClass, 
-                                                    discoveryFlaw);
-        }
-        else {
-            logDiagnostic("Could not instantiate Log "
-                          + logAdapterClassName + " -- "
-                          + discoveryFlaw.getLocalizedMessage());        
-        }
-    
-        
-        if (discoveryFlaw instanceof LogConfigurationException) {
-            throw (LogConfigurationException) discoveryFlaw;
-        }
-        else {
             throw new LogConfigurationException(discoveryFlaw);
         }
-        
     }
 
     
     /**
-     * Report a problem loading the log adapter, then return
+     * Report a problem loading the log adapter, then either return 
-     * a LogConfigurationException.
+     * (if the situation is considered recoverable) or throw a
+     * LogConfigurationException.
      *  <p>
      * There are two possible reasons why we successfully loaded the 
      * specified log adapter class then failed to cast it to a Log object:
@@ -849,58 +1056,86 @@ public class LogFactoryImpl extends LogFactory {
      * </ol>
      * <p>
      * Here we try to figure out which case has occurred so we can give the
-     *  user some reasonable feedback.
+     * user some reasonable feedback.
      * 
-     * @param logAdapterClass is the adapter class we successfully loaded (but which
+     * @param badClassLoader is the classloader we loaded the problem class from,
-     * could not be cast to type logInterface). Cannot be <code>null</code>.
+     * ie it is equivalent to badClass.getClassLoader().
      * 
-     * @param cause is the <code>Throwable</code> to wrap.
+     * @param badClass is a Class object with the desired name, but which 
+     * does not implement Log correctly.
      * 
-     * @return  <code>LogConfigurationException</code> that wraps 
+     * @throws LogConfigurationException when the situation
-     *          <code>cause</code> and includes a diagnostic message.
+     * should not be recovered from.
      */
-    private LogConfigurationException reportInvalidLogAdapter(
+    private void handleFlawedHierarchy(ClassLoader badClassLoader, Class badClass)
-            Class logAdapterClass, Throwable cause) {
+    throws LogConfigurationException {
-        
+
+        boolean implementsLog = false;
         String logInterfaceName = Log.class.getName();
-        Class interfaces[] = logAdapterClass.getInterfaces();
+        Class interfaces[] = badClass.getInterfaces();
         for (int i = 0; i < interfaces.length; i++) {
             if (logInterfaceName.equals(interfaces[i].getName())) {
-
+                implementsLog = true;
-                if (isDiagnosticsEnabled()) {                    
+                break;
-                    
-                    try {
-                        // Need to load the log interface so we know its
-                        // classloader for diagnostics
-                        ClassLoader logInterfaceClassLoader = getClassLoader(Log.class);
-                        ClassLoader logAdapterClassLoader = getClassLoader(logAdapterClass);
-                        Class logAdapterInterface = interfaces[i];
-                        ClassLoader logAdapterInterfaceClassLoader = getClassLoader(logAdapterInterface);
-                        logDiagnostic(
-                            "Class " + logAdapterClass.getName()
-                            + " was found in classloader " 
-                            + objectId(logAdapterClassLoader)
-                            + " but it implements the Log interface as loaded"
-                            + " from classloader " 
-                            + objectId(logAdapterInterfaceClassLoader)
-                            + " not the one loaded by this class's classloader "
-                            + objectId(logInterfaceClassLoader));
-                    } catch (Throwable t) {
-                        ;
-                    }
-                }
-                
-                return new LogConfigurationException
-                    ("Invalid class loader hierarchy.  " +
-                     "You have more than one version of '" +
-                     logInterfaceName + "' visible, which is " +
-                     "not allowed.", cause);
             }
         }
+        
+        if (implementsLog) {
+            // the class does implement an interface called Log, but
+            // it is in the wrong classloader
+            if (isDiagnosticsEnabled()) {
+                try {
+                    ClassLoader logInterfaceClassLoader = getClassLoader(Log.class);
+                    logDiagnostic(
+                        "Class " + badClass.getName()
+                        + " was found in classloader " 
+                        + objectId(badClassLoader)
+                        + ". It is bound to a Log interface which is not"
+                        + " the one loaded from classloader "
+                        + objectId(logInterfaceClassLoader));
+                } catch (Throwable t) {
+                    logDiagnostic(
+                        "Error while trying to output diagnostics about"
+                        + " bad class " + badClass);
+                }
+            }
             
-        return new LogConfigurationException
+            if (!allowFlawedHierarchy) {
-            ("Class " + logAdapterClass.getName() + " does not implement '" +
+                StringBuffer msg = new StringBuffer();
-                    logInterfaceName + "'.", cause);
+                msg.append("Terminating logging for this context ");
-    }
+                msg.append("due to bad log hierarchy. ");
+                msg.append("You have more than one version of ");
+                msg.append(Log.class.getName());
+                msg.append(" visible.");
+                logDiagnostic(msg.toString());
 
+                throw new LogConfigurationException(msg.toString());
+            }
+        
+            StringBuffer msg = new StringBuffer();
+            msg.append("Warning: bad log hierarchy. ");
+            msg.append("You have more than one version of ");
+            msg.append(Log.class.getName());
+            msg.append(" visible.");
+            logDiagnostic(msg.toString());
+        } else {
+            // this is just a bad adapter class
+            if (!allowFlawedDiscovery) {
+                StringBuffer msg = new StringBuffer();
+                msg.append("Terminating logging for this context. ");
+                msg.append("Log class ");
+                msg.append(badClass.getName());
+                msg.append(" does not implement the Log interface.");
+                logDiagnostic(msg.toString());
+
+                throw new LogConfigurationException(msg.toString());
+            }
+
+            StringBuffer msg = new StringBuffer();
+            msg.append("Warning: Log class ");
+            msg.append(badClass.getName());
+            msg.append(" does not implement the Log interface.");
+            logDiagnostic(msg.toString());
+        }
+    }
 }

src/test/org/apache/commons/logging/LoadTest.java

@@ -19,6 +19,8 @@ import junit.framework.Test;
 import junit.framework.TestCase;
 import junit.framework.TestSuite;
 
+import org.apache.commons.logging.impl.LogFactoryImpl;
+
 /**
  * testcase to emulate container and application isolated from container
  * @author  baliuka
@@ -97,7 +99,17 @@ public class LoadTest extends TestCase{
         
     }
     
-    
+
+    /**
+     * Call the static setAllowFlawedContext method on the specified class
+     * (expected to be a UserClass loaded via a custom classloader), passing
+     * it the specified state parameter.
+     */
+    private void setAllowFlawedContext(Class c, String state) throws Exception {
+        Class[] params = {String.class};
+        java.lang.reflect.Method m = c.getDeclaredMethod("setAllowFlawedContext", params);
+        m.invoke(null, new Object[] {state});
+    }
 
     /**
      * Test what happens when we play various classloader tricks like those
@@ -120,31 +132,50 @@ public class LoadTest extends TestCase{
         Thread.currentThread().setContextClassLoader(cls.getClassLoader());
         execute(cls);
         
-        // Context classloader is the "bootclassloader"
+        // Context classloader is the "bootclassloader". This is technically
+        // bad, but LogFactoryImpl.ALLOW_FLAWED_CONTEXT defaults to true so
+        // this test should pass.
         cls = reload();
         Thread.currentThread().setContextClassLoader(null);
         execute(cls);
         
+        // Context classloader is the "bootclassloader". This is same as above
+        // except that ALLOW_FLAWED_CONTEXT is set to false; an error should
+        // now be reported.
+        cls = reload();
+        Thread.currentThread().setContextClassLoader(null);
+        try {
+            setAllowFlawedContext(cls, "false");
+            execute(cls);
+            fail("Logging config succeeded when context classloader was null!");
+        } catch(LogConfigurationException ex) {
+            // expected; the boot classloader doesn't *have* JCL available
+        }
+        
         // Context classloader is the system classloader.
         //
         // This is expected to cause problems, as LogFactoryImpl will attempt
         // to use the system classloader to load the Log4JLogger class, which
         // will then be unable to cast that object to the Log interface loaded
-        // via the child classloader. JCL 1.0.4 and earlier will fail with
+        // via the child classloader. However as ALLOW_FLAWED_CONTEXT defaults
-        // this setup. Later versions of JCL should fail to load Log4J, but
+        // to true this test should pass.
-        // then fall back to jdk14 logging.
         cls = reload();
         Thread.currentThread().setContextClassLoader(ClassLoader.getSystemClassLoader());
         execute(cls);
         
-        // Context classloader is the classloader of this class, ie the
+        // Context classloader is the system classloader. This is the same
-        // parent classloader of the UserClass that will make the logging call.
+        // as above except that ALLOW_FLAWED_CONTEXT is set to false; an error 
-        //
+        // should now be reported.
-        // Actually, as the system classloader is expected to load this class,
-        // this test is identical to the preceding one.
         cls = reload();
-        Thread.currentThread().setContextClassLoader(this.getClass().getClassLoader());
+        Thread.currentThread().setContextClassLoader(ClassLoader.getSystemClassLoader());
-        execute(cls);
+        try {
+            setAllowFlawedContext(cls, "false");
+            execute(cls);
+            fail("Error: somehow downcast a Logger loaded via system classloader"
+                    + " to the Log interface loaded via a custom classloader");
+        } catch(LogConfigurationException ex) {
+            // expected 
+        }
     }
 
     /**
@@ -206,4 +237,15 @@ public class LoadTest extends TestCase{
         return suite;
     }
     
+    public void setUp() {
+        // save state before test starts so we can restore it when test ends
+        origContextClassLoader = Thread.currentThread().getContextClassLoader();
+    }
+    
+    public void tearDown() {
+        // restore original state so a test can't stuff up later tests.
+        Thread.currentThread().setContextClassLoader(origContextClassLoader);
+    }
+    
+    private ClassLoader origContextClassLoader;
 }

src/test/org/apache/commons/logging/NullClassLoaderTest.java

@@ -60,22 +60,8 @@ public class NullClassLoaderTest extends TestCase{
      * log object when called multiple times with the same name.
      */
     public void testSameLogObject() throws Exception {
-        ClassLoader oldContextClassLoader = Thread.currentThread().getContextClassLoader();
+        // unfortunately, there just isn't any way to emulate JCL being
-        try {
+        // accessable via the null classloader in "standard" systems, so
-            // emulate an app (not a webapp) running code loaded via the 
+        // we can't include this test in our standard unit tests.
-            // "null" classloader (bootclassloader for JDK1.2+, or
-            // systemclassloader for jdk1.1).
-            Thread.currentThread().setContextClassLoader(null);
-            
-            Log log1 = LogFactory.getLog("foo");
-            Log log2 = LogFactory.getLog("foo");
-            
-            assertSame(
-                "Calling getLog twice with the same category " +
-                "resulted in different objects!", 
-                log1, log2);
-        } finally {
-            Thread.currentThread().setContextClassLoader(oldContextClassLoader);
-        }
     }
 }

src/test/org/apache/commons/logging/UserClass.java

@@ -16,10 +16,25 @@
  
 package org.apache.commons.logging;
 
+import org.apache.commons.logging.LogFactory;
+import org.apache.commons.logging.impl.LogFactoryImpl;
+
 public class UserClass {
     
-   
+
- 
+    /**
+     * Set the ALLOW_FLAWED_CONTEXT feature on the LogFactoryImpl object
+     * associated with this class' classloader.
+     * <p>
+     * Don't forget to set the context classloader to whatever it will be
+     * when an instance of this class is actually created <i>before</i> calling
+     * this method!
+     */
+    public static void setAllowFlawedContext(String state) {
+        LogFactory f = LogFactory.getFactory();
+        f.setAttribute(LogFactoryImpl.ALLOW_FLAWED_CONTEXT_PROPERTY, state); 
+    }
+
     public UserClass() {
         Log log = LogFactory.getLog(LoadTest.class);
       }