Skip to content

fix: permit special characters in LDAP filter values - #30

Open
Alphix wants to merge 4 commits into
api7:mainfrom
Alphix:filter-fixes
Open

fix: permit special characters in LDAP filter values#30
Alphix wants to merge 4 commits into
api7:mainfrom
Alphix:filter-fixes

Conversation

@Alphix

@Alphix Alphix commented May 27, 2026

Copy link
Copy Markdown

This PR fixes some incorrect assertions which prohibits filters like (member=uid=john,ou=people,dc=example,dc=com).

Closes: #28

Summary by CodeRabbit

  • Bug Fixes
    • LDAP filter parsing now correctly treats RFC‑4515 assertion values (including =, <, >, ~) as literal content, fixing incorrect interpretation of special characters and improving related error diagnostics.
  • Tests
    • Added test coverage validating assertion values with special characters and a compound filter case.

Review Change Stack

Alphix and others added 3 commits May 27, 2026 15:38
to_hex is only used in two error-path format strings; a one-liner
gsub covers it, removing the undeclared external dependency. The
dependency was suggested in PR api7#7, but ultimately dropped.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
ATTRIBUTE_DESCRIPTION and ATTRIBUTE_VALUE both used RAW_VALUE, which
excludes =, <, >, ~ because they are filter operators. That exclusion
is correct for attribute descriptions but wrong for assertion values:
RFC 4515 UTF1SUBSET (%x01-27 / %x2B-5B / %x5D-7F) permits all of
them. Add a separate ASSERTION_VALUE rule using the RFC-compliant
ranges and switch ATTRIBUTE_VALUE to it.

Fixes: api7#28

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Now that ATTRIBUTE_VALUE uses ASSERTION_VALUE (which includes ~ via
its RFC 4515 ranges), UTF8_FILTERED_CHARACTER is only reached through
ATTRIBUTE_DESCRIPTION. Attribute descriptions are LDAP attribute names
and never contain ~, so the Cmt block was dead code. Remove it along
with the Cmt and string_sub locals it was the sole consumer of.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@CLAassistant

CLAassistant commented May 27, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@coderabbitai

coderabbitai Bot commented May 27, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

This PR internalizes the client's hex-encoding helper and updates the LDAP filter grammar to parse RFC 4515 assertion values, allowing operator characters (=, <, >, ~) inside attribute values; tests for these assertion values were added.

Changes

LDAP Module Updates

Layer / File(s) Summary
Client hex encoding localization
lib/resty/ldap/client.lua
The to_hex import from resty.string is removed and replaced with a local function that formats each byte as a two-digit lowercase hex value for error diagnostics.
Filter grammar RFC 4515 assertion value parsing
lib/resty/ldap/filter.lua, t/filter.t
LPEG constructors updated (added C, Ct, Cg, Cp, Cc, Cf; removed Cmt), ATTRIBUTE_VALUE now uses ASSERTION_VALUE; added ASSERTION_VALUE rule supporting RFC 4515 UTF1SUBSET and multi-byte UTF-8; simplified UTF8_FILTERED_CHARACTER; tests (t/filter.t) add cases for assertion values containing =, <, >, ~ and adjust a minor whitespace in an existing assertion.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes


Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 error, 1 warning)

Check name Status Explanation Resolution
Security Check ❌ Error TLS certificate verification is disabled by default (ssl_verify defaults to false) at line 242, creating MITM vulnerability for LDAP connections using STARTTLS or LDAPS. Change default ssl_verify from false to true or require explicit specification to prevent MITM attacks on unverified TLS connections.
E2e Test Quality Review ⚠️ Warning Tests are unit-only (filter.compile), not E2E. Per criteria: tests must cover full business flow with external dependencies, not isolated parsing. Add E2E tests validating LDAP queries with special-character filters (e.g., member=uid=john,...) against real LDAP server, not just unit tests.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: permitting special characters in LDAP filter values. This aligns with the PR's core objective of fixing RFC 4515 compliance for assertion values.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Alphix Alphix changed the title fix: improve LDAP filter fix: permit special characters in LDAP filter values May 28, 2026
Cover the fix from 6ac795b: RFC 4515 UTF1SUBSET permits =, <, >, ~ in
assertion values. Add TEST 5 with DN-style values (member=uid=john,...),
single-character embeddings of each operator, and a DN value inside a
compound filter.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented May 28, 2026

Copy link
Copy Markdown

Actionable comments posted: 0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unable to use filters like 'member=uid=jprjr,ou=people...'

2 participants