From c495408e8453f686b9c3f21407324edb08f043c2 Mon Sep 17 00:00:00 2001 From: Mattias Arthursson Date: Sun, 26 Oct 2008 18:41:22 +0000 Subject: [PATCH] Changed default of pooling flag in AbstractContextSource. Added configuration property for referral handling in AbstractContextSource. Updated changelog accordingly. --- changelog.txt | 9 ++++ .../control/PagedResultsRequestControl.java | 7 +++ .../core/support/AbstractContextSource.java | 43 +++++++++++++++---- ...ctTlsDirContextAuthenticationStrategy.java | 7 +++ .../core/support/LdapContextSourceTest.java | 2 +- ...SourceEc2InstanceLaunchingFactoryBean.java | 14 ++++-- .../conf/ldapTemplateTestContext.xml | 15 +++---- 7 files changed, 77 insertions(+), 20 deletions(-) diff --git a/changelog.txt b/changelog.txt index c1656c83..7718e99c 100644 --- a/changelog.txt +++ b/changelog.txt @@ -46,6 +46,15 @@ Changes in version 1.3 (XXX 2008) system property org.springframework.ldap.core.spacedDnFormat to true. (LDAP-138, LDAP-112, LDAP-91) +* Changed default of 'pooling' flag in AbstractContextSource. + This now defaults to false; consider the Spring LDAP PoolingContextSource + as the preferred alternative to using the built-in Java Connection Pooling. + +* Added configuration property to AbstractContextSource to specify referral + behavior. Setting this property to 'follow' will enable referrals to be + automatically followed, provieded that the name server environment is properly + set up. + * Now using Maven for building internally. (LDAP-80, LDAP-82, LDAP-95) * Added HardcodedFilter class and corresponding PropertyEditor FilterEditor, diff --git a/core/src/main/java/org/springframework/ldap/control/PagedResultsRequestControl.java b/core/src/main/java/org/springframework/ldap/control/PagedResultsRequestControl.java index bc3786ac..b7add92b 100644 --- a/core/src/main/java/org/springframework/ldap/control/PagedResultsRequestControl.java +++ b/core/src/main/java/org/springframework/ldap/control/PagedResultsRequestControl.java @@ -29,6 +29,13 @@ import java.lang.reflect.Method; /** * DirContextProcessor implementation for managing the paged results control. + * Note that due to the internal workings of LdapTemplate, the + * target connection is closed after each LDAP call. The PagedResults control + * require the same connection be used for each call, which means we need to + * make sure the target connection is never actually closed. There's basically + * two ways of making this happen: use the SingleContextSource + * implementation or make sure all calls happen within a single LDAP transaction + * (using ContextSourceTransactionManager). * * @author Mattias Hellborg Arthursson * @author Ulrik Sandberg diff --git a/core/src/main/java/org/springframework/ldap/core/support/AbstractContextSource.java b/core/src/main/java/org/springframework/ldap/core/support/AbstractContextSource.java index a1b38de0..93c9d3ec 100644 --- a/core/src/main/java/org/springframework/ldap/core/support/AbstractContextSource.java +++ b/core/src/main/java/org/springframework/ldap/core/support/AbstractContextSource.java @@ -80,7 +80,7 @@ public abstract class AbstractContextSource implements BaseLdapPathContextSource private String[] urls; - private boolean pooled = true; + private boolean pooled = false; private Hashtable baseEnv = new Hashtable(); @@ -92,6 +92,8 @@ public abstract class AbstractContextSource implements BaseLdapPathContextSource private boolean anonymousReadOnly = false; + private String referral = null; + private static final Log log = LogFactory.getLog(AbstractContextSource.class); public static final String SUN_LDAP_POOLING_FLAG = "com.sun.jndi.ldap.connect.pool"; @@ -357,6 +359,10 @@ public abstract class AbstractContextSource implements BaseLdapPathContextSource env.put(Context.OBJECT_FACTORIES, dirObjectFactory.getName()); } + if (!StringUtils.isBlank(referral)) { + env.put(Context.REFERRAL, referral); + } + if (!DistinguishedName.EMPTY_PATH.equals(base)) { // Save the base path for use in the DefaultDirObjectFactory. env.put(DefaultDirObjectFactory.JNDI_ENV_BASE_PATH_KEY, base); @@ -416,13 +422,20 @@ public abstract class AbstractContextSource implements BaseLdapPathContextSource } /** - * Set whether the pooling flag should be set. Default is true. Note that - * since LDAP pooling is system wide, full configuration of this needs be - * done using system parameters as specified in the LDAP/JNDI documentation. - * Also note, that pooling is done on user dn basis, i.e. each individually - * authenticated connection will be pooled separately. This means that LDAP - * pooling will be most efficient using anonymous connections or connections - * authenticated using one single system user. + * Set whether the pooling flag should be set, enabling the built-in LDAP + * connection pooling. Default is false. The built-in LDAP + * connection pooling suffers from a number of deficiencies, e.g. no + * connection validation. Also, enabling this flag when using TLS + * connections will explicitly not work. Consider using the Spring LDAP + * PoolingContextSource as an alternative instead of enabling + * this flag. + *

+ * Note that since LDAP pooling is system wide, full configuration of this + * needs be done using system parameters as specified in the LDAP/JNDI + * documentation. Also note, that pooling is done on user dn basis, i.e. + * each individually authenticated connection will be pooled separately. + * This means that LDAP pooling will be most efficient using anonymous + * connections or connections authenticated using one single system user. * * @param pooled whether Contexts should be pooled. */ @@ -538,6 +551,20 @@ public abstract class AbstractContextSource implements BaseLdapPathContextSource this.authenticationStrategy = authenticationStrategy; } + /** + * Set the method to handle referrals. Default is 'ignore'; setting this + * flag to 'follow' will enable referrals to be automatically followed. Note + * that this might require particular name server setup in order to work + * (the referred URLs will need to be automatically found using standard DNS + * resolution). + * @param referral the value to set the system property + * Context.REFERRAL to, customizing the way that referrals are + * handled. + */ + public void setReferral(String referral) { + this.referral = referral; + } + /** * Implement in subclass to create a DirContext of the desired type (e.g. * InitialDirContext or InitialLdapContext). diff --git a/core/src/main/java/org/springframework/ldap/core/support/AbstractTlsDirContextAuthenticationStrategy.java b/core/src/main/java/org/springframework/ldap/core/support/AbstractTlsDirContextAuthenticationStrategy.java index 106e649b..1f00390d 100755 --- a/core/src/main/java/org/springframework/ldap/core/support/AbstractTlsDirContextAuthenticationStrategy.java +++ b/core/src/main/java/org/springframework/ldap/core/support/AbstractTlsDirContextAuthenticationStrategy.java @@ -52,6 +52,13 @@ import org.springframework.ldap.support.LdapUtils; * For further information regarding TLS, refer to this * page. + *

+ * NB: TLS negotiation is an expensive process, which is why you will + * most likely want to use connection pooling, to make sure new connections are + * not created for each individual request. It is imperative however, that the + * built-in LDAP connection pooling is not used in combination with the TLS + * AuthenticationStrategy implementations - this will not work. You should use + * the Spring LDAP PoolingContextSource instead. * * @author Mattias Hellborg Arthursson */ diff --git a/core/src/test/java/org/springframework/ldap/core/support/LdapContextSourceTest.java b/core/src/test/java/org/springframework/ldap/core/support/LdapContextSourceTest.java index a0369483..e14aa6c8 100644 --- a/core/src/test/java/org/springframework/ldap/core/support/LdapContextSourceTest.java +++ b/core/src/test/java/org/springframework/ldap/core/support/LdapContextSourceTest.java @@ -119,7 +119,7 @@ public class LdapContextSourceTest extends TestCase { tested.afterPropertiesSet(); Hashtable env = tested.getAnonymousEnv(); assertEquals("ldap://ldap.example.com:389", env.get(Context.PROVIDER_URL)); - assertEquals("true", env.get(LdapContextSource.SUN_LDAP_POOLING_FLAG)); + assertNull(env.get(LdapContextSource.SUN_LDAP_POOLING_FLAG)); } public void testGetAnonymousEnvWithPoolingInBaseEnvironmentAndPoolingOff() throws Exception { diff --git a/test-support/src/main/java/org/springframework/ldap/test/ContextSourceEc2InstanceLaunchingFactoryBean.java b/test-support/src/main/java/org/springframework/ldap/test/ContextSourceEc2InstanceLaunchingFactoryBean.java index 9a2ec4fb..5a4384fa 100644 --- a/test-support/src/main/java/org/springframework/ldap/test/ContextSourceEc2InstanceLaunchingFactoryBean.java +++ b/test-support/src/main/java/org/springframework/ldap/test/ContextSourceEc2InstanceLaunchingFactoryBean.java @@ -21,7 +21,8 @@ import org.springframework.ldap.test.AbstractEc2InstanceLaunchingFactoryBean; import org.springframework.util.Assert; /** - * FactoryBean to create a ContextSource using the EC2 instance created by superclass. + * FactoryBean to create a ContextSource using the EC2 instance created by + * superclass. */ public class ContextSourceEc2InstanceLaunchingFactoryBean extends AbstractEc2InstanceLaunchingFactoryBean { @@ -31,6 +32,8 @@ public class ContextSourceEc2InstanceLaunchingFactoryBean extends AbstractEc2Ins private String password; + private boolean pooled = false; + @Override public final Class getObjectType() { return ContextSource.class; @@ -38,18 +41,23 @@ public class ContextSourceEc2InstanceLaunchingFactoryBean extends AbstractEc2Ins @Override protected final Object doCreateInstance(final String dnsName) throws Exception { - Assert.hasText(userDn); + Assert.hasText(userDn); LdapContextSource instance = new LdapContextSource(); instance.setUrl("ldap://" + dnsName); instance.setUserDn(userDn); instance.setPassword(password); instance.setBase(base); + instance.setPooled(pooled); setAdditionalContextSourceProperties(instance, dnsName); instance.afterPropertiesSet(); return instance; } + public void setPooled(boolean pooled) { + this.pooled = pooled; + } + /** * Override to set additional properties on the ContextSource. * @@ -57,7 +65,7 @@ public class ContextSourceEc2InstanceLaunchingFactoryBean extends AbstractEc2Ins * @param dnsName The dns name of the created Ec2 instance. */ protected void setAdditionalContextSourceProperties(LdapContextSource ctx, final String dnsName) { - //Nothing to do here + // Nothing to do here } public void setBase(String base) { diff --git a/test/integration-tests-openldap/src/test/resources/conf/ldapTemplateTestContext.xml b/test/integration-tests-openldap/src/test/resources/conf/ldapTemplateTestContext.xml index cc69d3ae..84db7a05 100644 --- a/test/integration-tests-openldap/src/test/resources/conf/ldapTemplateTestContext.xml +++ b/test/integration-tests-openldap/src/test/resources/conf/ldapTemplateTestContext.xml @@ -3,13 +3,13 @@ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans-2.0.xsd"> - - + - - + @@ -18,10 +18,9 @@ + - - + - + \ No newline at end of file