Hi,
On 20/09/2010 20:08, Sebastien Bahloul wrote:
Hi guys,
Here is a simple patch to include a wrapper for "%20" to " " in base context
name. If not included, context dn including "%20" are not wrapped correctly
and try to delete the entry through it's full dn (dawn jndi !)
Before commiting it, does someone have any comment about this crappy patch ?
Sure, I have some comments on the patch, thanks for posting it :)
But first, why is there not a bug open on this subject? It clearly looks
like a bug to me (especially if it causes entries to be deleted
wrongly!). I already asked Quentin several times to create one on IRC.
This bug is actually related to a fix I wrote recently (see bug #209).
If I had seen a bug report I could have fixed it long ago...
So, about the patch, a few comments:
- Replacing "%20" is not sufficient - there are many special characters
that are encoded in LDAP URLs, which all need to be decoded for this
comparison to take place.
- The contextDN attribute is used similarly in other places in the
JndiServices class - you would need to patch those too, but I think this
patch is in the wrong place.
I didn't see a test in your patch, which would be nice to understand
what's actually happening, and make sure we don't break it in the
future. I attach a possible test, can you confirm this is indeed the
issue you have?
If so, there is no problem in 1.3 branch or trunk according to this
test. I attach a proposed patch to fix the issue in 1.2 (basically a
backport from 1.3/trunk). Does that work for you?
Jonathan
--
--------------------------------------------------------------
Jonathan Clarke - [email protected]
--------------------------------------------------------------
Ldap Synchronization Connector (LSC) - http://lsc-project.org
--------------------------------------------------------------
Index: src/test/java/org/lsc/jndi/JndiServicesTest.java
===================================================================
--- src/test/java/org/lsc/jndi/JndiServicesTest.java (révision 836)
+++ src/test/java/org/lsc/jndi/JndiServicesTest.java (copie de travail)
@@ -555,5 +555,22 @@
assertEquals(1, sr.size());
assertEquals("cn=test", sr.get(0));
}
+
+ /**
+ * testRewriteBaseWithSpaces()
+ */
+ @Test
+ public final void testRewriteBaseWithSpaces() throws NamingException, IOException {
+ Properties props = (Properties) Configuration.getDstProperties().clone();
+ JndiServices jndiServices;
+ List<String> sr;
+
+ props.put("java.naming.provider.url", "ldap://localhost:33389/o=bla bla,dc=lsc-project,dc=org");
+ jndiServices = JndiServices.getInstance(props);
+
+ String res = jndiServices.rewriteBase("uid=test,o=bla bla,dc=lsc-project,dc=org");
+ assertEquals("uid=test", res);
+ }
+
}
Index: src/main/java/org/lsc/jndi/JndiServices.java
===================================================================
--- src/main/java/org/lsc/jndi/JndiServices.java (révision 836)
+++ src/main/java/org/lsc/jndi/JndiServices.java (copie de travail)
@@ -87,6 +87,7 @@
import org.lsc.Configuration;
import org.lsc.LscAttributes;
+import com.unboundid.ldap.sdk.DN;
import com.unboundid.ldap.sdk.LDAPException;
import com.unboundid.ldap.sdk.LDAPURL;
@@ -209,7 +210,13 @@
}
/* handle options */
- contextDn = namingContext.getDN() != null ? namingContext.getDN() : null;
+ try {
+ contextDn = new LDAPURL((String) ctx.getEnvironment().get(Context.PROVIDER_URL)).getBaseDN().toString();
+ } catch (LDAPException e) {
+ LOGGER.error(e.toString());
+ LOGGER.debug(e.toString(), e);
+ throw new NamingException(e.getMessage());
+ }
String pageSizeStr = (String) ctx.getEnvironment().get("java.naming.ldap.pageSize");
if (pageSizeStr != null) {
_______________________________________________________________
Ldap Synchronization Connector (LSC) - http://lsc-project.org
lsc-dev mailing list
[email protected]
http://lists.lsc-project.org/listinfo/lsc-dev
