diff --git a/changelog.txt b/changelog.txt index e9538c35..a6d976b1 100644 --- a/changelog.txt +++ b/changelog.txt @@ -18,6 +18,9 @@ Changes in version 1.3.0 (Nov 2008) and optionally callback to a supplied instance to perform an LDAP operation on the context. +* Re-used the same fix for LDAP-109 and LDAP-50 that was used in DefaultDirObjectFactory to secure + the DirContextAdapter constructors from invalid CompositeNames. + Changes in version 1.3.0.RC1 (Oct 2008) ------------------------------------------- * TLS connections are now supported using the DefaultTlsDirContextAuthenticationStrategy diff --git a/core/src/main/java/org/springframework/ldap/core/DirContextAdapter.java b/core/src/main/java/org/springframework/ldap/core/DirContextAdapter.java index 95c36857..4358c2c3 100644 --- a/core/src/main/java/org/springframework/ldap/core/DirContextAdapter.java +++ b/core/src/main/java/org/springframework/ldap/core/DirContextAdapter.java @@ -149,13 +149,13 @@ public class DirContextAdapter implements DirContextOperations { this.originalAttrs = new BasicAttributes(true); } if (dn != null) { - this.dn = new DistinguishedName(dn.toString()); + this.dn = new DistinguishedName(dn); } else { this.dn = new DistinguishedName(); } if (base != null) { - this.base = new DistinguishedName(base.toString()); + this.base = new DistinguishedName(base); } else { this.base = new DistinguishedName(); diff --git a/core/src/main/java/org/springframework/ldap/core/DistinguishedName.java b/core/src/main/java/org/springframework/ldap/core/DistinguishedName.java index 92a2959f..d78159a8 100644 --- a/core/src/main/java/org/springframework/ldap/core/DistinguishedName.java +++ b/core/src/main/java/org/springframework/ldap/core/DistinguishedName.java @@ -33,7 +33,9 @@ import org.apache.commons.lang.StringUtils; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.ldap.BadLdapGrammarException; +import org.springframework.ldap.support.LdapUtils; import org.springframework.ldap.support.ListComparator; +import org.springframework.util.Assert; /** * Default implementation of a {@link Name} corresponding to an LDAP path. A @@ -158,6 +160,11 @@ public class DistinguishedName implements Name { * DistinguishedName from. */ public DistinguishedName(Name name) { + Assert.notNull(name, "name cannot be null"); + if (name instanceof CompositeName) { + parse(LdapUtils.convertCompositeNameToString((CompositeName) name)); + return; + } names = new LinkedList(); for (int i = 0; i < name.size(); i++) { names.add(new LdapRdn(name.get(i))); diff --git a/core/src/main/java/org/springframework/ldap/core/LdapEntryIdentificationContextMapper.java b/core/src/main/java/org/springframework/ldap/core/LdapEntryIdentificationContextMapper.java index f53f6d94..a5aaf1ac 100644 --- a/core/src/main/java/org/springframework/ldap/core/LdapEntryIdentificationContextMapper.java +++ b/core/src/main/java/org/springframework/ldap/core/LdapEntryIdentificationContextMapper.java @@ -15,8 +15,6 @@ */ package org.springframework.ldap.core; -import org.springframework.ldap.core.support.AbstractContextMapper; - /** * ContextMapper implementation that maps the found entries to the * {@link LdapEntryIdentification} of each respective entry. @@ -24,11 +22,11 @@ import org.springframework.ldap.core.support.AbstractContextMapper; * @author Mattias Hellborg Arthursson * @since 1.3 */ -public class LdapEntryIdentificationContextMapper extends AbstractContextMapper { - - protected Object doMapFromContext(DirContextOperations ctx) { - return new LdapEntryIdentification(new DistinguishedName(ctx.getNameInNamespace()), new DistinguishedName(ctx - .getDn())); - } +public class LdapEntryIdentificationContextMapper implements ContextMapper { + public Object mapFromContext(Object ctx) { + DirContextOperations adapter = (DirContextOperations) ctx; + return new LdapEntryIdentification(new DistinguishedName(adapter.getNameInNamespace()), new DistinguishedName( + adapter.getDn())); + } } diff --git a/core/src/main/java/org/springframework/ldap/core/support/DefaultDirObjectFactory.java b/core/src/main/java/org/springframework/ldap/core/support/DefaultDirObjectFactory.java index f079eb92..294e1c90 100644 --- a/core/src/main/java/org/springframework/ldap/core/support/DefaultDirObjectFactory.java +++ b/core/src/main/java/org/springframework/ldap/core/support/DefaultDirObjectFactory.java @@ -30,6 +30,7 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.springframework.ldap.core.DirContextAdapter; import org.springframework.ldap.core.DistinguishedName; +import org.springframework.ldap.support.LdapUtils; import org.springframework.util.StringUtils; /** @@ -122,19 +123,14 @@ public class DefaultDirObjectFactory implements DirObjectFactory { // problem. CompositeName.toString() completely screws up the // formatting // in some cases, particularly when backslashes are involved. - CompositeName compositeName = (CompositeName) name; - if (compositeName.size() > 0) { - // A lookup with an empty String seems to produce an empty - // compositeName here; need to take this into account. - nameString = compositeName.get(0); - } - else { - nameString = ""; - } + nameString = LdapUtils + .convertCompositeNameToString((CompositeName) name); } else { - log.warn("Expecting a CompositeName as input to getObjectInstance but received a '" - + name.getClass().toString() + "' - using toString and proceeding with undefined results"); + log + .warn("Expecting a CompositeName as input to getObjectInstance but received a '" + + name.getClass().toString() + + "' - using toString and proceeding with undefined results"); nameString = name.toString(); } diff --git a/core/src/main/java/org/springframework/ldap/support/LdapUtils.java b/core/src/main/java/org/springframework/ldap/support/LdapUtils.java index 99ef99ef..cbae2118 100644 --- a/core/src/main/java/org/springframework/ldap/support/LdapUtils.java +++ b/core/src/main/java/org/springframework/ldap/support/LdapUtils.java @@ -18,6 +18,7 @@ package org.springframework.ldap.support; import java.util.Collection; +import javax.naming.CompositeName; import javax.naming.directory.Attribute; import javax.naming.directory.Attributes; import javax.naming.directory.DirContext; @@ -290,4 +291,23 @@ public final class LdapUtils { collection.add(attributeValue); } } + + /** + * Converts a CompositeName to a String in a way that avoids escaping + * problems, such as the dreaded "triple backslash" problem. + * + * @param compositeName The CompositeName to convert + * @return String containing the String representation of name + */ + public static String convertCompositeNameToString( + CompositeName compositeName) { + if (compositeName.size() > 0) { + // A lookup with an empty String seems to produce an empty + // compositeName here; need to take this into account. + return compositeName.get(0); + } + else { + return ""; + } + } } diff --git a/core/src/test/java/org/springframework/ldap/core/DirContextAdapterTest.java b/core/src/test/java/org/springframework/ldap/core/DirContextAdapterTest.java index f3f79181..61816a9d 100644 --- a/core/src/test/java/org/springframework/ldap/core/DirContextAdapterTest.java +++ b/core/src/test/java/org/springframework/ldap/core/DirContextAdapterTest.java @@ -19,6 +19,7 @@ package org.springframework.ldap.core; import java.util.Iterator; import java.util.SortedSet; +import javax.naming.CompositeName; import javax.naming.Name; import javax.naming.NamingException; import javax.naming.directory.Attribute; @@ -35,6 +36,7 @@ import junit.framework.TestCase; * * @author Andreas Ronge * @author Mattias Hellborg Arthursson + * @author Ulrik Sandberg */ public class DirContextAdapterTest extends TestCase { private static final DistinguishedName BASE_NAME = new DistinguishedName("dc=jayway, dc=se"); @@ -1070,4 +1072,17 @@ public class DirContextAdapterTest extends TestCase { ModificationItem[] modificationItems = tested.getModificationItems(); assertEquals(0, modificationItems.length); } + + /** + * Test for LDAP-109, since also DirContextAdapter may get an invalid + * CompositeName sent to it. + */ + public void testConstructorUsingCompositeNameWithBackslashes() + throws Exception { + CompositeName compositeName = new CompositeName(); + compositeName.add("cn=Some\\\\Person6,ou=company1,c=Sweden"); + DirContextAdapter adapter = new DirContextAdapter(compositeName); + assertEquals("cn=Some\\\\Person6,ou=company1,c=Sweden", adapter.getDn() + .toString()); + } } diff --git a/core/src/test/java/org/springframework/ldap/core/DistinguishedNameTest.java b/core/src/test/java/org/springframework/ldap/core/DistinguishedNameTest.java index 93ab6b7e..9542ff04 100644 --- a/core/src/test/java/org/springframework/ldap/core/DistinguishedNameTest.java +++ b/core/src/test/java/org/springframework/ldap/core/DistinguishedNameTest.java @@ -37,6 +37,12 @@ import com.gargoylesoftware.base.testing.EqualsTester; public class DistinguishedNameTest extends TestCase { public void testDistinguishedName_CompositeWithSlash() throws Exception { + Name testPath = new CompositeName("cn=foo\\/bar"); + DistinguishedName path = new DistinguishedName(testPath); + assertEquals("cn=foo/bar", path.toString()); + } + + public void testDistinguishedName_CompositeWithSlashAsString() throws Exception { Name testPath = new CompositeName("cn=foo\\/bar"); DistinguishedName path = new DistinguishedName(testPath.toString()); assertEquals("cn=foo/bar", path.toString());