OPENDJ-2776 Remove insignificant/illegal whitespace during parsing of DNs

Matthew Swift

16.17.2016 ac272266185cd7aefee60209b4fcfd47733d4d15

OPENDJ-2776 Remove insignificant/illegal whitespace during parsing of DNs

Strictly speaking RFC 4514 does not allow whitespace before or after RDN
or AVA separators. Whilst it is often good practice to be tolerant of
malformed input, we should not produce it.

The DN class was tolerating whitespace characters around the RDN and AVA
separators, but was not removing them, causing the toString() method to
product an invalid string representation.

This change provides a simple fix: do not cache the user provided
string, but instead regenerate it lazily when toString() is called.
Whilst the fix is simple it does incur a slight performance penalty in
the common case where the input DN string is valid.

 opendj-sdk/opendj-core/src/main/java/org/forgerock/opendj/ldap/DN.java

@@ -60,7 +60,7 @@
    static final char RDN_CHAR_SEPARATOR = ',';
    static final char AVA_CHAR_SEPARATOR = '+';

    private static final DN ROOT_DN = new DN(CoreSchema.getInstance(), null, null, "");
    private static final DN ROOT_DN = new DN(CoreSchema.getInstance(), null, null);

    /**
     * This is the size of the per-thread per-schema DN cache. We should
@@ -238,8 +238,7 @@
        }

        // Not in cache so decode.
        final SubstringReader reader = new SubstringReader(dn);
        return decode(dn, reader, schema, cache);
        return decode(new SubstringReader(dn), schema, cache);
    }

    /**
@@ -258,8 +257,7 @@
    }

    /** Decodes a DN using the provided reader and schema. */
    private static DN decode(final String dnString, final SubstringReader reader,
            final Schema schema, final Map<String, DN> cache) {
    private static DN decode(final SubstringReader reader, final Schema schema, final Map<String, DN> cache) {
        reader.skipWhitespaces();
        if (reader.remaining() == 0) {
            return ROOT_DN;
@@ -282,7 +280,7 @@
            parent = cache.get(parentString);
            if (parent == null) {
                reader.reset();
                parent = decode(parentString, reader, schema, cache);
                parent = decode(reader, schema, cache);

                // Only cache parent DNs since leaf DNs are likely to make the
                // cache to volatile.
@@ -292,7 +290,7 @@
            parent = ROOT_DN;
        }

        return new DN(schema, parent, rdn, dnString);
        return new DN(schema, parent, rdn);
    }

    @SuppressWarnings("serial")
@@ -322,27 +320,24 @@
     */
    private ByteString normalizedDN;

    /**
     * We need to store the original string value if provided in order to
     * preserve the original whitespace.
     */
    /** The RFC 4514 string representation of this DN. */
    private String stringValue;

    /** The schema used to create this DN. */
    private final Schema schema;

    /** Private constructor. */
    private DN(final Schema schema, final DN parent, final RDN rdn, final String stringValue) {
        this(schema, parent, rdn, stringValue, parent != null ? parent.size + 1 : 0);
    private DN(final Schema schema, final DN parent, final RDN rdn) {
        this(schema, parent, rdn, parent != null ? parent.size + 1 : 0);
    }

    /** Private constructor. */
    private DN(final Schema schema, final DN parent, final RDN rdn, final String stringValue, final int size) {
    private DN(final Schema schema, final DN parent, final RDN rdn, final int size) {
        this.schema = schema;
        this.parent = parent;
        this.rdn = rdn;
        this.stringValue = stringValue;
        this.size = size;
        this.stringValue = rdn == null ? "" : null; // Compute lazily.
    }

    /**
@@ -370,7 +365,7 @@
            }
            DN newDN = this;
            for (i = 0; i < rdns.length; i++) {
                newDN = new DN(this.schema, newDN, rdns[i], null);
                newDN = new DN(this.schema, newDN, rdns[i]);
            }
            return newDN;
        }
@@ -394,7 +389,7 @@
     */
    public DN child(final RDN rdn) {
        Reject.ifNull(rdn);
        return new DN(this.schema, this, rdn, null);
        return new DN(this.schema, this, rdn);
    }

    /**
@@ -741,11 +736,11 @@
        } else if (index >= size) {
            return this;
        } else {
            final DN localName = new DN(this.schema, null, rdn, null, index);
            final DN localName = new DN(this.schema, null, rdn, index);
            DN nextLocalName = localName;
            DN lastDN = parent;
            for (int i = index - 1; i > 0; i--) {
                nextLocalName.parent = new DN(this.schema, null, lastDN.rdn, null, i);
                nextLocalName.parent = new DN(this.schema, null, lastDN.rdn, i);
                nextLocalName = nextLocalName.parent;
                lastDN = lastDN.parent;
            }
@@ -989,7 +984,7 @@
        private final Schema schema;

        private CompactDn(final DN dn) {
            this.originalValue = dn.stringValue != null ? getBytes(dn.stringValue) : new byte[0];
            this.originalValue = getBytes(dn.toString());
            this.schema = dn.schema;
        }


 opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldap/DNTestCase.java

@@ -1246,4 +1246,24 @@
        UUID uuid2 = DN.valueOf("cn=test+dc=example,dc=com").toUUID();
        assertEquals(uuid1, uuid2);
    }

    @DataProvider
    public Object[][] toStringShouldStripOutIllegalWhitespaceDataProvider() {
        // @formatter:off
        return new Object[][] {
            { " ", "" },
            { " dc = hello  world ", "dc=hello  world" },
            { " dc =\\  hello  world\\  ", "dc=\\  hello  world\\ " },
            { " dc = example , dc = com ", "dc=example,dc=com" },
            { " uid = crystal + dc = example , uid = palace + dc = com ", "uid=crystal+dc=example,uid=palace+dc=com" },
        };
        // @formatter:on
    }

    @Test(dataProvider = "toStringShouldStripOutIllegalWhitespaceDataProvider")
    public void toStringShouldStripOutIllegalWhitespace(String withWhiteSpace, String withoutWhiteSpace) {
        assertThat(DN.valueOf(withWhiteSpace).toString()).isEqualTo(withoutWhiteSpace);
        assertThat(DN.valueOf(withWhiteSpace).toNormalizedByteString())
                .isEqualTo(DN.valueOf(withoutWhiteSpace).toNormalizedByteString());
    }
}

 opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldif/LDIFChangeRecordReaderTestCase.java

@@ -11,7 +11,7 @@
 * Header, with the fields enclosed by brackets [] replaced by your own identifying
 * information: "Portions Copyright [year] [name of copyright owner]".
 *
 * Copyright 2011 ForgeRock AS.
 * Copyright 2011-2016 ForgeRock AS.
 * Portions copyright 2012 ForgeRock AS.
 * Portions Copyright 2014 Manuel Gaupp
 */
@@ -1122,8 +1122,7 @@
        // Read the record
        ChangeRecord record = reader.readChangeRecord();
        assertThat(record).isInstanceOf(DeleteRequest.class);
        assertThat(record.getName().toString()).isEqualTo(
                "ou=Product Development, dc=airius, dc=com");
        assertThat(record.getName().toString()).isEqualTo("ou=Product Development,dc=airius,dc=com");
        assertThat(record.getControls()).isNotEmpty();
        assertThat(record.getControls().get(0).getOID()).isEqualTo("1.2.840.113556.1.4.805");
        assertThat(record.getControls().get(0).getValue().toString()).isEqualTo("cn");
@@ -1154,8 +1153,7 @@
        // Read the record
        ChangeRecord record = reader.readChangeRecord();
        assertThat(record).isInstanceOf(AddRequest.class);
        assertThat(record.getName().toString()).isEqualTo(
                "ou=Product Development, dc=airius, dc=com");
        assertThat(record.getName().toString()).isEqualTo("ou=Product Development,dc=airius,dc=com");
        assertThat(record.getControls()).isNotEmpty();
        assertThat(record.getControls().get(0).getOID()).isEqualTo("1.3.6.1.1.13.1");
        assertThat(record.getControls().get(0).getValue().toString()).isEqualTo("cn");
@@ -1213,8 +1211,7 @@
        // Read the record
        ChangeRecord record = reader.readChangeRecord();
        assertThat(record).isInstanceOf(AddRequest.class);
        assertThat(record.getName().toString()).isEqualTo(
                "ou=Product Development, dc=airius, dc=com");
        assertThat(record.getName().toString()).isEqualTo("ou=Product Development,dc=airius,dc=com");
        assertThat(record.getControls()).isNotEmpty();
        assertThat(record.getControls().get(0).getOID()).isEqualTo("1.3.6.1.1.13.1");
        assertThat(record.getControls().get(0).getValue().toString()).isEqualTo("cn");
@@ -1245,8 +1242,7 @@
        // Read the record
        ChangeRecord record = reader.readChangeRecord();
        assertThat(record).isInstanceOf(AddRequest.class);
        assertThat(record.getName().toString()).isEqualTo(
                "ou=Product Development, dc=airius, dc=com");
        assertThat(record.getName().toString()).isEqualTo("ou=Product Development,dc=airius,dc=com");
        assertThat(record.getControls()).isNotEmpty();
        assertThat(record.getControls().get(0).getOID()).isEqualTo("1.3.6.1.1.13.1");
        assertThat(record.getControls().get(0).getValue().toString()).isEqualTo("sn");
@@ -1389,8 +1385,7 @@
        // Read the record
        ChangeRecord record = reader.readChangeRecord();
        assertThat(record).isInstanceOf(AddRequest.class);
        assertThat(record.getName().toString()).isEqualTo(
                "ou=Product Development, dc=airius, dc=com");
        assertThat(record.getName().toString()).isEqualTo("ou=Product Development,dc=airius,dc=com");
        assertThat(record.getControls()).isNotEmpty();
        assertThat(record.getControls().get(0).getOID()).isEqualTo("1.3.6.1.1.13.1");
        assertThat(record.getControls().get(0).getValue()).isNull();
@@ -1421,8 +1416,7 @@
        // Read the record
        ChangeRecord record = reader.readChangeRecord();
        assertThat(record).isInstanceOf(AddRequest.class);
        assertThat(record.getName().toString()).isEqualTo(
                "ou=Product Development, dc=airius, dc=com");
        assertThat(record.getName().toString()).isEqualTo("ou=Product Development,dc=airius,dc=com");
        assertThat(record.getControls()).isNotEmpty();
        assertThat(record.getControls().get(0).getOID()).isEqualTo("1.3.6.1.1.13.1");
        assertThat(record.getControls().get(0).getValue().toString()).isEqualTo("description");
@@ -1500,24 +1494,21 @@
        reader.setSchemaValidationPolicy(SchemaValidationPolicy.defaultPolicy());
        // 1st
        ChangeRecord record = reader.readChangeRecord();
        assertThat(record.getName().toString()).isEqualTo(
                "ou=Product Development, dc=airius, dc=com");
        assertThat(record.getName().toString()).isEqualTo("ou=Product Development,dc=airius,dc=com");
        assertThat(record.getControls()).isNotEmpty();
        assertThat(record.getControls().get(0).getOID()).isEqualTo("1.3.6.1.1.13.1");
        assertThat(record.getControls().get(0).getValue()).isNull();
        //2nd
        record = reader.readChangeRecord();
        assertThat(record).isInstanceOf(AddRequest.class);
        assertThat(record.getName().toString()).isEqualTo(
                "cn=Paul Jensen, ou=Product Development, dc=airius, dc=com");
        assertThat(record.getName().toString()).isEqualTo("cn=Paul Jensen,ou=Product Development,dc=airius,dc=com");
        assertThat(record.getControls()).isNotEmpty();
        assertThat(record.getControls().get(0).getOID()).isEqualTo("1.3.6.1.1.13.13");
        assertThat(record.getControls().get(0).getValue().toString()).isEqualTo("cn");
        //3rd
        record = reader.readChangeRecord();
        assertThat(record).isInstanceOf(AddRequest.class);
        assertThat(record.getName().toString()).isEqualTo(
                "cn=Paula Jensen, ou=Product Development, dc=airius, dc=com");
        assertThat(record.getName().toString()).isEqualTo("cn=Paula Jensen,ou=Product Development,dc=airius,dc=com");
        assertThat(record.getControls()).isNotEmpty();
        assertThat(record.getControls().get(0).getOID()).isEqualTo("1.3.6.1.1.13.13.16");
        assertThat(record.getControls().get(0).getValue().toString()).isEqualTo("sn");
@@ -1525,8 +1516,7 @@
    }

    /**
     * Test to read an record containing an invalid control. (pair.value is
     * null) Must throw a DecodeException.
     * Test to read an record containing an invalid control. (pair.value is null) Must throw a DecodeException.
     *
     * @throws Exception
     */

 opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldif/LDIFChangeRecordWriterTestCase.java

@@ -11,7 +11,7 @@
 * Header, with the fields enclosed by brackets [] replaced by your own identifying
 * information: "Portions Copyright [year] [name of copyright owner]".
 *
 * Copyright 2011-2015 ForgeRock AS.
 * Copyright 2011-2016 ForgeRock AS.
 * Portions copyright 2012 ForgeRock AS.
 */

@@ -823,8 +823,7 @@
        writer.writeChangeRecord(changeRequest);
        writer.close();

        assertThat(actual.get(0))
                .isEqualTo("dn: cn=Robert Jensen, ou=Marketing, dc=airius, dc=com");
        assertThat(actual.get(0)).isEqualTo("dn: cn=Robert Jensen,ou=Marketing,dc=airius,dc=com");
        assertThat(actual.get(1)).isEqualTo("changetype: delete");
    }


 opendj-sdk/opendj-core/src/test/java/org/forgerock/opendj/ldif/LDIFEntryReaderTestCase.java

@@ -12,7 +12,7 @@
 * information: "Portions Copyright [year] [name of copyright owner]".
 *
 * Copyright 2009-2010 Sun Microsystems, Inc.
 * Portions copyright 2012 ForgeRock AS.
 * Portions copyright 2012-2016 ForgeRock AS.
 */

package org.forgerock.opendj.ldif;
@@ -1105,33 +1105,27 @@
        // @formatter:on
        final String path = TestCaseUtils.createTempFile(strEntry);
        final FileInputStream in = new FileInputStream(path);
        final LDIFEntryReader reader = new LDIFEntryReader(in);

        Entry entry = null;
        try {
        try (LDIFEntryReader reader = new LDIFEntryReader(in)) {
            assertThat(reader.hasNext());
            // 1st entry
            entry = reader.readEntry();
            assertThat(entry.getName().toString()).isEqualTo(
                    "cn=Barbara Jensen, ou=Product Development, dc=airius, dc=com");
            Entry entry = reader.readEntry();
            assertThat(entry.getName()
                            .toString()).isEqualTo("cn=Barbara Jensen,ou=Product Development,dc=airius,dc=com");
            assertThat(entry.getAttributeCount()).isEqualTo(6);
            // 2nd
            entry = reader.readEntry();
            assertThat(entry.getName().toString()).isEqualTo(
                    "cn=Bjorn Jensen, ou=Accounting, dc=airius, dc=com");
            assertThat(entry.getName().toString()).isEqualTo("cn=Bjorn Jensen,ou=Accounting,dc=airius,dc=com");
            assertThat(entry.getAttributeCount()).isEqualTo(4);

            assertThat(reader.hasNext()).isFalse();
        } finally {
            reader.close();
        }
    }


    /**
     * Tries to read an entry composed by multi-valued attributes. The
     * multi-valued attributes contains an interesting case where two of them
     * represents the same value, one in uppercase and the other in lower case.
     * Tries to read an entry composed by multi-valued attributes. The multi-valued attributes contains an interesting
     * case where two of them represents the same value, one in uppercase and the other in lower case.
     *
     * @throws Exception
     */