-
Notifications
You must be signed in to change notification settings - Fork 9.2k
HADOOP-18603 NPE in LdapAuthenticationHandler as disableHostNameVerif… #5581
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
Conversation
|
💔 -1 overall
This message was automatically generated. |
|
Could you please add a unit test that fails without your change and passes with the change. |
ayushtkn
left a comment
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.
I am not sure why it is Boolean object rather than a primitive boolean, but the fix looks fine to me.
How difficult it to extend a test for this?
This change was part of HADOOP-12082. it shows @lmccay as a participant. @lmccay any thoughts/concerns on this?
…ication is never initialized.
bb7c1ea to
15a4a26
Compare
|
Thank you for the review @ayushtkn @simbadzina. I have added testcase for this. |
|
🎊 +1 overall
This message was automatically generated. |
ayushtkn
left a comment
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.
Should be fine.
@simbadzina any comments?
| * Constant for disabling the host name verification for this handler. | ||
| */ | ||
| private static final String DISABLE_HOSTNAME_VERIFICATION = TYPE + | ||
| ".hostname.verification.disable"; |
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.
Could you please also add this new config in core-site default.
Requested a change to document new config.
|
@simbadzina None of the configs added in this file are added to core-default.xml. I'm thinking we can plan to add it as part of new jira. Would it be ok? Thanks |
|
We're closing this stale PR because it has been open for 100 days with no activity. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable. |
…ication is never initialized.