Support DN to string conversion #154

[asambeka]

Added String method to dn.go, for supporting DN to string conversion.

[liggitt]

where is escaping done?

[asambeka]

Value inspection and escaping (RFC 2253, 4514) is not implemented. Only canonical representation with spaces removed from [=,+] was done. I can add value inspection and escaping.

[asambeka]

@liggitt I have added the changes for handing escaping of attribute values as per RFC 4512, 4514. Implementation is generic at DN level and can be used easily.

[asambeka]

@liggitt Here is the change (EscapeAttrValue), please review.

[asambeka]

@liggitt Can you please provide your review inputs on this? - Thnx

[asambeka]

@liggitt Please check and approve the PR

[johnweldon]

Thank you

[johnweldon]

@asambeka - I really appreciate your work on this. I have a couple reservations that incline me to want to use #104 instead of this PR:

1. The other PR adds the encoding to the AttributeTypeAndValue type directly rather than in a loop on the containing type. I prefer keeping the encoding close to the type that actually is affected by it.
2. This PR uses regular expressions, rather than the simple parse loop in the other PR. I feel that regular expression use for this is overkill and less user friendly for understanding what the code is doing.
3. The other PR handles encoding hex values, which this PR neglects to do.

I am sorry I didn't review the differences more thoroughly before you worked on the updates today. Have I missed any benefits of this PR over the other one that I did not enumerate?

[asambeka]

1. Having String() for AttributeTypeValue is a good choice.
2. RegEx vs hand-parsing was a choice I made for readability, I am certain there will be other opinions. Ideally this should be via lexical analyzer/compiler.. this is how it was handled traditionally by LDAPs. I am okay with hand parsing though.
3. Hex encoding: I need to do little bit more analysis on this. I'm bit skeptical about hex encoding being different in DN Object and String forms. Please note, this is value that is received from LDAP server (or user), so it should already be in WYSIWYG form. IMHO we are not aligned with other implementations (JNDI/Mozilla C-SDK)

Here are few red flags from #104 for me, I think these makes this SDK non compliant:

a) PR is explicitly sorting multi-valued RNDs. LDAPs do not modify DN Inserted by user, they will preserve the format. Sorting is done only for “DN Normalization” which is typically needed for DN compare operation (which does not modify DN). So String() should not sort.
b) PR is changing case as well. There is no need to lower the case in String(). Lower is needed for canonical/Normalization use case only (when doing DN compare etc). DN is case in-sensitive, but it does not mean that the case can be changed.

Realization that a DN and Normalized value is same is done by Matching Rules (in this case DN Matching Rule). Matching rules are immutable, String() should be no different.

Sorting and lower case functionality #104 is implementing should be under "DN Normalization".

[johnweldon]

I'm inclined to agree that lowercasing and sorting do not belong in String() but maybe in NormalizedString()

I would still rather see a parser than a regular expression.

I would be interested in your analysis of the hex encoding. It seems important to do it right, although the String() method is really for display, not for actual over-the-wire encoding.

[johnweldon]

Feel free to reopen or make a new PR if you'd like to pursue this further. Thanks for you contribution.