search

inejge / ldap3

A pure-Rust LDAP library using the Tokio stack
Apache License 2.0
220 stars 38 forks source link

Update to a more recent version of nom #92

Closed lwagner94 closed 1 year ago

lwagner94 commented 1 year ago

Hey there!

We are currently evaluating ldap3 because we are interested in using it in Proxmox Backup Server to enable LDAP login for our users. So far, we are pretty happy with it. As we try to keep the number of duplicated/outdated dependencies to a minimum, we were wondering: Are there currently any plans to update to a more recent version of nom?

Best Regards, Lukas

inejge commented 1 year ago

Are there currently any plans to update to a more recent version of nom?

Now there are :slightly_smiling_face: More seriously, I've long been aware of the age of that particular dependency, but I've postponed upgrading due to internal nom changes in the transitions between v3 and v4, and then again between v4 and v5, particularly the handling of non-streaming input. Your question tells me that there is outside interest in upgrading the parser.

One thing that is potentially more involved than upgrading nom in the main crate is doing the same in lber, which is very old code, practically untouched since the crate was forked. I don't want to upgrade hastily, since both lber and filter parsing are foundational to the functionality of the crate. Again, now that I know that there is interest, I can look into it seriously.

lwagner94 commented 1 year ago

Great! Thank you for your reply and for considering our request. :+1:

Regarding lber, I don't know if I have missed something, but it actually seemed to be the easier upgrade from what I have seen. The parser in src/parse.rs is pretty small and I dont see any other uses of nom apart from that file.

dequbed commented 1 year ago

Regarding lber, I don't know if I have missed something, but it actually seemed to be the easier upgrade from what I have seen. The parser in src/parse.rs is pretty small and I dont see any other uses of nom apart from that file.

It's a bit more annoying as nom has since version 2 removed some of the partial parsing statekeeping it provided users with (notably Consumer and Move) but anyway, #93 exists now ¯\_(ツ)_/¯

inejge commented 1 year ago

The lber part: merged, as referenced above.

The filter part: merged, see 99fd7ddb7cb358b0634102c724e04e8bb23941e4.

You can test with the current HEAD, and I'm going to publish a 0.11 beta when I finish a couple of other housekeeping tasks. I'm leaving the issue open until then.

lwagner94 commented 1 year ago

Fantastic! Thank you very much. I'll give this a try and let you know if I run into any issues.

inejge commented 1 year ago

0.11-beta has been published; closing this issue.

lwagner94 commented 1 year ago

Thanks again to both of you for your efforts. I've been testing 0.11-beta for a while now and could not find any issues :+1: FYI: https://lists.proxmox.com/pipermail/pbs-devel/2023-January/005788.html