-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
[release/6.0] Fixing regression for AuthType.Anonymous which leads to a NullReferenceException be thrown. #62824
Conversation
Tagging subscribers to this area: @dotnet/area-system-directoryservices, @jay98014 |
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.
LGTM
src/libraries/System.DirectoryServices.Protocols/src/System.DirectoryServices.Protocols.csproj
Show resolved
Hide resolved
var connection = new LdapConnection("server"); | ||
connection.AuthType = AuthType.Anonymous; | ||
// When calling Bind we make sure that the exception thrown is not that there was a NullReferenceException | ||
// trying to retrive a null password's lenght, but instead an LdapException given the server cannot be reached. |
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.
Just noticed a typo here: lenght
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.
good catch, will fix in main
Backport of #62807 to release/6.0
/cc @joperezr
Customer Impact
This PR is fixing a customer reported regression on this issue: #61683
The regression is present only on NuGet package System.DirectoryServices.Protocols version 6.0.0
The problem here is that if a consumer is running on Linux, and tries to Bind to a LDAP server using
AuthType.Anonymous
then we will throw aNullReferenceException
due to a bug we have where we try to de-reference the user's password which in Anonymous case, will be null. There is no good workaround for this for using Anonymous AuthType today, other than downgrading the NuGet package version to 5.0.1 which doesn't have the regression.Testing
We've added unit test for regression coverage, and the fix has been validated by the customer that first reported the issue and said it fixed the issue for them. (cc. @JasonDebug)
Risk
Low. This is fixing a regression from 5.0->6.0 and currently we have no good workaround to avoid throwing NullReferenceException. The risks here are basically the same as shipping a new version of any of our NuGet packages.