-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Fix LDAP network timeout not being honored on Linux #119210
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
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: ericstj <8918108+ericstj@users.noreply.github.com>
Co-authored-by: ericstj <8918108+ericstj@users.noreply.github.com>
Co-authored-by: ericstj <8918108+ericstj@users.noreply.github.com>
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
This comment was marked as off-topic.
src/libraries/System.DirectoryServices.Protocols/tests/LdapConnectionTests.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.DirectoryServices.Protocols/tests/LdapConnectionTests.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.DirectoryServices.Protocols/tests/LdapConnectionTests.cs
Show resolved
Hide resolved
src/libraries/System.DirectoryServices.Protocols/tests/LdapConnectionTests.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.DirectoryServices.Protocols/tests/LdapConnectionTests.cs
Show resolved
Hide resolved
...ectoryServices.Protocols/src/System/DirectoryServices/Protocols/ldap/LdapConnection.Linux.cs
Outdated
Show resolved
Hide resolved
@copilot can you address the feedback? |
Co-authored-by: ericstj <8918108+ericstj@users.noreply.github.com>
Co-authored-by: ericstj <8918108+ericstj@users.noreply.github.com>
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
src/libraries/System.DirectoryServices.Protocols/tests/LdapConnectionTests.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes a bug where System.DirectoryServices.Protocols.LdapConnection
does not honor the Timeout
property at the network layer on Linux, potentially causing network operations to hang indefinitely. The solution implements proper network-level timeout support for Linux by setting LDAP_OPT_NETWORK_TIMEOUT
via ldap_set_option
.
- Added
LDAP_OPT_NETWORK_TIMEOUT
constant and P/Invoke support for setting timeval structures - Modified
InternalConnectToServer
in Linux implementation to set network timeout using theTimeout
property - Added comprehensive cross-platform tests to verify timeout behavior
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
src/libraries/System.DirectoryServices.Protocols/tests/LdapConnectionTests.cs | Added unit tests for timeout functionality including unreachable server test |
src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/ldap/LdapConnection.Linux.cs | Modified InternalConnectToServer to set network timeout after URI configuration |
src/libraries/System.DirectoryServices.Protocols/src/System/DirectoryServices/Protocols/Interop/LdapPal.Linux.cs | Added SetTimevalOption method for setting timeval-based LDAP options |
src/libraries/Common/src/Interop/Linux/OpenLdap/Interop.Ldap.cs | Added P/Invoke declaration for ldap_set_option_timeval |
src/libraries/Common/src/Interop/Interop.Ldap.cs | Added LDAP_OPT_NETWORK_TIMEOUT constant to LdapOption enum |
...ectoryServices.Protocols/src/System/DirectoryServices/Protocols/ldap/LdapConnection.Linux.cs
Outdated
Show resolved
Hide resolved
…rectoryServices/Protocols/ldap/LdapConnection.Linux.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Fixes #62075
Problem
On Linux,
System.DirectoryServices.Protocols.LdapConnection
does not honor theTimeout
property at the network layer, causing network operations (such as connect or bind) to potentially hang indefinitely or much longer than the requested timeout. This occurs because the native LDAP handle does not haveLDAP_OPT_NETWORK_TIMEOUT
set, while on Windows the timeout is properly enforced at the network layer.Solution
This PR implements proper network-level timeout support for Linux by setting
LDAP_OPT_NETWORK_TIMEOUT
vialdap_set_option
on the LDAP handle after initialization and before any bind or search operations.Key Changes
LDAP_OPT_NETWORK_TIMEOUT
constant (0x5005) to theLdapOption
enumldap_set_option_timeval
InternalConnectToServer
inLdapConnection.Linux.cs
to set network timeout using theTimeout
property valueCheckAndSetLdapError
for timeout option settingImplementation Details
The timeout is set in
InternalConnectToServer
after successful URI configuration:The implementation uses standard error handling patterns consistent with other LDAP options in the codebase.
Testing
Impact
This change brings Linux behavior in line with Windows and makes the
Timeout
property work as documented across platforms. While this could change behavior for callers whose connections previously took longer than the default timeout (30s) without overriding it, this is a desirable bug fix that improves reliability and cross-platform consistency.Fixes scenarios where LDAP connections would hang indefinitely on Linux when network connectivity issues occurred, particularly important for applications that rely on predictable timeout behavior.
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.