noirello / bonsai

Simple Python 3 module for LDAP, using libldap2 and winldap C libraries.
MIT License
116 stars 32 forks source link

LDIFError - "Invalid attribute value pair: '{attrval}' for entry #{self.__num_of_entries}." #72

Closed kpopovic closed 1 year ago

kpopovic commented 1 year ago

Hello,

I have and issue with LDIFReader It throws an error in case of postaladdress. As it can be seen it contains colon in P.O.Box: This causes ldif.py to break in method def __next__(self) -> LDAPEntry:

dn: vfsid=491723666666,ou=subscriber,ou=MMO,c=de,o=telecom
sn: Europe Generation
cn: Europe Generation
postalcode: 03182
postaladdress: P.O.Box: 20 01 40 03182

This will return error ValueError: too many values to unpack (expected 2)

elif ": " in attrval:
    attr, val = attrval.split(": ") # tuple of three ['postaladdress', 'P.O.Box', '20 01 40 03182'] 
    # will throw ValueError: too many values to unpack (expected 2)
...
except:
   except ValueError:
          raise LDIFError(
              f"Invalid attribute value pair: '{attrval}' for entry #{self.__num_of_entries}."

I use bonsai==1.5.0.

rra commented 1 year ago

Mentioning this here just to save someone else the research: I had thought that LDIF was invalid and values containing : had to be base64-encoded, but it looks from RFC 2849 like I'm wrong and colons are allowed in values as long as the colon isn't the first character. I think it's generally recommended to base64-encode such values, but since it can be unambiguously parsed, it's apparently not required.

kpopovic commented 1 year ago

Hello,

I have adapted my code to work with ldif file I have.

But maybe small recommendation for the future, improvement:

Maybe to improve parsing in case of ": "

elif ": " in attrval:
    attr, val = attrval.split(": ", 1) # maxsplit=1, instead of 0
...
except:
 raise LDIFError(
      f"Invalid attribute value pair: '{attrval}' for entry #{self.__num_of_entries}."
      ) from None

Or just to improve error description:

except Exception as ex:
 raise LDIFError(
      f"Invalid attribute value pair: '{attrval}' for entry #{self.__num_of_entries}., {ex}"
      ) from None # better description why error happend...
noirello commented 1 year ago

Hi, thanks for reporting it. I added both of your recommendations:

You can see the changes in the linked commits

noirello commented 1 year ago

The fix has been release in 1.5.1.