Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LDAP-229: Default DN parser does not handle hash/sharp symbol correctly #274

Closed
spring-projects-issues opened this issue May 18, 2011 · 8 comments
Milestone

Comments

@spring-projects-issues
Copy link

spring-projects-issues commented May 18, 2011

Pavel Horal (Migrated from LDAP-229) said:

DnParserImpl incorrectly terminates DN parsing, when it encounters # symbol.

System.out.println(new DistinguishedName("cn=Foo#Bar,cn=Test")); // cn=Foo

According to http://tools.ietf.org/html/rfc4514#section-4 hash/sharp symbol which does not appear at the beginning of the attribute value should be handled as a regular string character.

@spring-projects-issues
Copy link
Author

spring-projects-issues commented May 18, 2011

Pavel Horal said:

Hash/sharp symbol not being in the set of string characters is probably the cause:

TOKEN: { <#STRINGCHAR: ~[",","=","+","<",">","#",";","\\","\""]> }

However RFC 4514 states these to be string characters:

stringchar = SUTF1 / UTFMB
SUTF1 = %x01-21 / %x23-2A / %x2D-3A /
        %x3D / %x3F-5B / %x5D-7F

String definition in the RFC 4514 contains the # symbol in the second group %x23-2A.

@spring-projects-issues
Copy link
Author

spring-projects-issues commented May 18, 2011

Ulrik Sandberg said:

Hash/sharp symbol not being in the set of string characters is probably the cause:

TOKEN: { <#STRINGCHAR: ~[",","=","+","<",">","#",";","\\","\""]> }

Not sure what you're suggesting. Hash (#) is in STRINGCHAR.

It looks OK, since ESCAPEDSTART seems to cover it:

TOKEN: { <#ESCAPEDSTART: <BACKSLASHCHAR> (<HASHCHAR> | <SPACE>)> }
...
        <ATTRVALUE:
                (<QUOTECHAR> (<STRINGCHAR> | <SPECIAL> | <PAIR>)+ <QUOTECHAR>
                |
                <HASHCHAR> (<HEXPAIR>)+
                |
                (<ESCAPEDSTART>)? ( <STRINGCHAR> | <PAIR> )* <STRINGEND>
                )>

Do you have a unit test that triggers this?

@spring-projects-issues
Copy link
Author

spring-projects-issues commented May 19, 2011

Pavel Horal said:

Not sure what you're suggesting. Hash (#) is in STRINGCHAR.

Can you please double-check your/my statement. All I can see is STRINGCHAR being defined as a set of all Unicode character with specific set of excluded symbols. The # (hash) symbol is 6th excluded symbol.

It looks OK, since ESCAPEDSTART seems to cover it:

ESCAPEDSTART does not cover this, because it is covering only the case when the # (hash) symbol is in the beginning of the attribute value where it MUST be escaped. If the # (hash) symbol is not in the beginning of the attribute value it does not have to be escaped (defined in RFC 4514).

Do you have a unit test that triggers this?

The single line of code in the bug description can reproduce the issue. Do you require a real unit test?

@spring-projects-issues
Copy link
Author

spring-projects-issues commented May 19, 2011

Pavel Horal said:

RFC link in the bug description contains invalid section reference - correct is http://tools.ietf.org/html/rfc4514#section-3 .

By the way there are also other characters which are incorrectly excluded from the STRINGCHAR - e.g. = (equals).

@spring-projects-issues
Copy link
Author

spring-projects-issues commented May 19, 2011

Pavel Horal said:

Test case for the hash parsing.

@spring-projects-issues
Copy link
Author

spring-projects-issues commented May 23, 2011

Pavel Horal said:

Some additional information - the Spring LDAP parser is apparently only LDAPv2 compliant (http://tools.ietf.org/html/rfc2253#section-3 ).

However there were made some significant DN syntax changes for LDAPv3 which are summarized at http://tools.ietf.org/html/rfc4514#appendix-B :

  • did not require escaping of equals sign ('=' U+003D) characters,
  • did not require escaping of non-leading number sign ('#' U+0023) characters,
  • allowed space (' ' U+0020) to be escaped as '\ ',
  • required hex escaping of null (U+0000) characters, and
  • removed LDAPv2-only constructs.

This means that Spring LDAP is not fully compliant to LDAPv3 with its current parser implementation.

@spring-projects-issues
Copy link
Author

spring-projects-issues commented May 23, 2011

Pavel Horal said:

I've updated DnParserImpl.jj (see the attached patch) so it is compliant with LDAPv3 (i.e. RFC 4514).

Only problem is that now it is not fully backward compatible because LDAPv3 does not define attribute values enclosed within double quotes. However in the Spring LDAP project there is no unit test for such functionality. Existing unit tests are working with the new parser.

The correct solution might be to have two parsers (one LDAPv2 and one LDAPv3) and allow developers switching between them (e.g. via JVM property). I also suggest to add new unit tests for unescaped hash, space and equals sign within an attribute value.

For Spring LDAP users with the same problem, I am also uploading archive with generated parser sources (see the attached archive). You can include them within your project to override the default implementation.

@spring-projects-issues
Copy link
Author

spring-projects-issues commented Aug 12, 2013

Mattias Hellborg Arthursson said:

Applied the suggested patch and test cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant