Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Relax validation of openldap_database's suffix parameter #402

Merged
merged 1 commit into from Jan 6, 2024

Conversation

smortex
Copy link
Member

@smortex smortex commented Jan 5, 2024

The validation of the suffix parameter assumed conformance with
RFC2247, but this is not a requirement and cause trouble to some users.

A full validation of the suffix syntax to check if it conforms to
RFC2253 is probably overkill, so for now, we just relax the used regexp
to allow valid values which where previously rejected.

Some valid DN syntax are still rejected, as supporting them would
require to replace the regexp with a parser. Add them as known-bad
examples in the test suite (pending tests).

Fixes #391
Fixes #396

The validation of the `suffix` parameter assumed conformance with
RFC2247, but this is not a requirement and cause trouble to some users.

A full validation of the suffix syntax to check if it conforms to
RFC2253 is probably overkill, so for now, we just relax the used regexp
to allow valid values which where previously rejected.

Some valid DN syntax are still rejected, as supporting them would
require to replace the regexp with a parser.  Add them as known-bad
examples in the test suite (pending tests).

Fixes #391
Fixes #396
@smortex smortex added the bug Something isn't working label Jan 5, 2024
@smortex
Copy link
Member Author

smortex commented Jan 5, 2024

@virus2500 can you please give this a try/review this PR? A "clean" fix is an unreasonable amount of work, but this quick fix should solve your issue 😉

@smortex smortex marked this pull request as ready for review January 5, 2024 02:19
@virus2500
Copy link

@smortex Just tested it with an running install and an fresh install. Everything is working as expected. Thank you for your work :)

@smortex
Copy link
Member Author

smortex commented Jan 6, 2024

Thanks! I'll ask for review in case somebody spot something wrong and will do a release soon.

@smortex smortex merged commit 8a7f124 into master Jan 6, 2024
28 checks passed
@smortex smortex deleted the relax-dn-syntax branch January 6, 2024 01:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
3 participants