Support DN to string conversion #154#155
Conversation
dn.go
Outdated
| if j > 0 { | ||
| buffer.WriteString("+") | ||
| } | ||
| buffer.WriteString(d.RDNs[i].Attributes[j].Type) |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
@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.
dn.go
Outdated
| buffer.WriteString(d.RDNs[i].Attributes[j].Type) | ||
| buffer.WriteString("=") | ||
| //Escape the value before building DN string | ||
| val := EscapeAttrValue(d.RDNs[i].Attributes[j].Value) |
|
@liggitt Please check and approve the PR |
|
@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:
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? |
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. 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". |
|
I'm inclined to agree that lowercasing and sorting do not belong in 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 |
|
Feel free to reopen or make a new PR if you'd like to pursue this further. Thanks for you contribution. |
Added String method to dn.go, for supporting DN to string conversion.