Attribute tagging interferes with entry.Unmarshal

[JesseCoretta]

Consider the following struct excerpt:

          Value  string `ldap:"attributeName"`
          CValue string `ldap:"c-attributeName"` // is COLLECTIVE

When processed through entry.Unmarshal, both COLLECTIVE and NON-COLLECTIVE values are unmarshaled as expected, assuming the attribute descriptors found within the search result(s) appear as shown in the configured struct tags.

However, in certain X.500/LDAP implementations, COLLECTIVE attributes are assigned the ;collective attribute tag in search results. In such cases, entry.Unmarshal fails to recognize attributes bearing this tag.

The work-around is simply:

          Value  string `ldap:"attributeName"`
          CValue string `ldap:"c-attributeName;collective"`  // replace the previous CValue definition with this one

In this case entry.Unmarshal recognizes the value and handles it appropriately.

Of course, the above "code edit" is likely not advisable in the real-world. In an ideal situation, the presence of either c-attributeName or c-attributeName;collective should produce the same unmarshaled results.

Feel free to ping me with any questions, thank you 😃

Jesse

EDIT: It is also worth mentioning that not all situations involving tags would apply to the above. For example, language tags:

givenName: Jesse
givenName;lang-jp: ジェシー

Even if they are essentially saying the same thing, the two values are in fact totally distinct. Requesting the tag, and not requesting the tag, would and should return differing results. Furthermore, the act of supporting multiple languages as shown above is entirely driven by the manner in which the directory was designed and is being governed/populated.

The ;collective scenario, however, is "DSA implementation specific" and is not something the user "chose to see": the software itself imposes this tag whether we wanted it or not.

Therefore, I would assume that IF a solution were to be implemented, it would be safest to limit its scope to the aforementioned ;collective use case specifically, but I could be wrong.

[JesseCoretta]

I have tested a potential fix, which I will outline below. I've also considered an alternative to this solution, which is also discussed.

Option 1 - A Simple "Fallback" Patch

Patch Synopsis

This patch implements attribute unmarshaling "fallback" in terms of the tag value(s) specified within a struct intended to undergo entry unmarshaling.

First, we split the attribute tag by pipe (ASCII dec. 124 / hex %7C). Honestly any character that is not a legal type;tag character would suffice. But somehow, pipe seemed to fit the best in the logical sense, considering the idea of a fallback in this context is essentially an "OR'ed statement".

Next, we iterate the split tag(s) and attempt to access values with which it is associated, and break-out of the matching process at the first positive match.

This solution is entirely user-elected and should not be a breaking change for those not using this functionality. Users who do not utilize this fallback strategy will enjoy the same results as before. However, in my case and others, the following will yield higher compatibility:

  PostalAddress string `ldap:"c-postalAddress|c-postalAddress;collective"`

In addition to addressing this issue, this patch also has the virtue of addressing corner-cases where a numeric OID is used in place of an attribute descriptor, for instance:

  PostalAddress string `ldap:"c-postalAddress|2.5.4.16.1"`

Granted, this was more common in the old days, but is still a valid consideration today. It is worth mentioning this is a current shortcoming of the unmarshaler, so I consider this secondary effect to be a bonus.

The patch -- which I tested starting at line 277 of search.go -- is crude but simple and effective within the terms of this issue. I'm really just spit-balling here, I'm not married to any particular strategy, so feel free to suggest changes. This is the same reason I did not simply create a pull request, since I'm not a member of the team and felt this may warrant discussions first.

   fieldTags := strings.Split(fieldTag,`|`) // pipe split

   var values []string
   for _, subtag := range fieldTags {
           if values = e.GetAttributeValues(subtag); len(values) != 0 {
                   // Found a tag match; discontinue loop.
                   break
           }
   }

   if len(values) == 0 {
           // NO tags were matched, or no values were found,
           // so do what go-ldap was originally doing and
           // abandon this iteration for the next one.
           continue
   }

Option 2 - Closure-based Unmarshaling

I only bring this option to light because it has the virtue of satisfying any user who wishes the entry unmarshaler supported more types in struct fields (e.g.: such as nested structs, as I personally would love).

Rather than appeasing all future requests for such alterations, I'm wondering if the dev team would be more amenable to writing a secondary Entry.Unmarshal method, for instance Entry.UnmarshalFunc.

In theory, such a method would allow an input closure (signature TBD) which enables the end-user to author their own unmarshaler using reflect or some other means.

This means the package would be capable of catering to all strata of end users, and would get the devs "off the hook" in terms of honoring any future "expansions" of the current unmarshaler.

I would be happy to devise a demo for this particular option, but because it is obviously more grandiose in scope, I figured I would wait to see people's reactions.

Please, anyone, feel free to chime in.

Jesse

[JesseCoretta]

@cpuschma - has there been any feedback from your team regarding either (or both) of the above approaches? I am willing to create a PR for this, but was looking for some guidance as to which of these options is more palatable.

Thanks

Jesse

[cpuschma]

@JesseCoretta You are right. I would tend towards option 2, as this also offers users the most flexibility in the long term. I have prepared a PR and would be happy to receive feedback if you have the time! #557

[JesseCoretta]

Thanks @cpuschma -- I will be testing this today.

[JesseCoretta]

@cpuschma -- so far, my tests have been successful.

Use cases:

• Tag deconstruction (e.g.: recognizing either "c-l" OR "c-l;collective" via pipe-splitting)
• Descending recursion (e.g.: field is pointer to struct -- initialize instance if needed and continue populating fields by self-executing the entry.UnmarshalFunc method)

[cpuschma]

PR has been merged. Thanks again for your help @JesseCoretta ❤️

[JesseCoretta]

PR has been merged. Thanks again for your help @JesseCoretta ❤️

Thanks @cpuschma, nicely done as always 😺