Skip to content

fix: make LDAP operation completion and result states unambiguous #33

Description

@membphis

Context

This is a post-merge production-readiness follow-up to #32. It is intentionally kept out of the current refactor PR so that the receive-state and public-result contracts can be designed and tested together.

At head b665ffa91a2671e81158e4c1a445530442b8bc7e, _send_receive treats a non-timeout failure while reading the next response header as end-of-stream. For a multi-response Search, the server can therefore close after zero or more entries but before SearchResultDone, and the accumulated table can be returned as a successful empty or partial result.

The decoder exposes message_id, but the client does not correlate every response with the request in flight or handle messageID=0 as a separate unsolicited notification. RFC 4511 requires responses to carry the corresponding request message ID, reserves zero for unsolicited notifications, and defines a Search result as zero or more entries/references followed by exactly one SearchResultDone:

Goal

Make every successful client operation prove that it received the expected terminal response for the exact request, and expose a stable machine-readable outcome contract that downstream callers can consume without parsing error strings.

Required behavior

  • Track the expected non-zero request messageID and reject any response with a different ID.
  • Handle messageID=0 notifications, including Notice of Disconnection, as connection-level events rather than responses to the current request.
  • Require the expected terminal response for each operation. In particular, Search succeeds only after one matching SearchResultDone; EOF is never a substitute for completion.
  • Treat closed, EOF, RST, timeout, truncated header/body, unexpected operation, duplicate terminal response, and missing terminal response as failures. Invalidate and close the socket so it cannot re-enter a pool or contaminate the next request.
  • Expose enough structured information for callers to distinguish at least invalid credentials, other LDAP result codes, timeout, transport/TLS failure, and protocol failure. Do not require callers to classify failures by matching human-readable strings.
  • Preserve and document the compatibility contract of ldap_authenticate, including its canonical escaped DN return, while adding any structured result needed by downstream integrations.

Acceptance criteria

  • A Search that receives entries and then EOF/RST before SearchResultDone returns an error, never partial success.
  • A Search that receives no entries and closes before SearchResultDone returns an error, never empty success.
  • Wrong-ID responses before or during Bind/Search, an ID-zero Notice of Disconnection, a wrong terminal operation, duplicate SearchResultDone, and truncated headers/bodies all fail closed and close the socket.
  • Both pinned and single-operation clients are covered, including a regression proving that a failed exchange cannot affect a subsequent request or pooled connection.
  • Tests cover valid zero-entry Search, valid multi-entry Search, server LDAP result errors, invalid credentials, timeout, transport/TLS failure, and protocol failure through the public API.
  • Public return/error semantics and compatibility behavior are documented.

Out of scope

APISIX-specific HTTP status mapping, Consumer lookup, Basic credential parsing, log redaction, and product configuration belong in a separate apache/apisix issue after this library contract is agreed.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions