feat(NODE-2939): update hostname canonicalization opts #3129
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
addaleax
reviewed
Feb 4, 2022
| function resolveCname(host: string, callback: Callback<string>): void { | ||
| // Attempt to resolve the host name | ||
| dns.resolveCname(host, (err, r) => { | ||
| if (err) return callback(err); |
There was a problem hiding this comment.
I think this should also be
Suggested change
| if (err) return callback(err); | |
| if (err) return callback(undefined, host); |
because Node.js gives you ENODATA errors for domains which exist but have no CNAME records attached
5 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Updates
CANONICALIZE_HOST_NAMEoptions to accepttrue,false, "none", "forward", "forwardAndReverse".What is changing?
When canonicalizing the host name when using GSSAPI, the driver will now behave as follows for these values:
trueor "forwardAndReverse": Performs a forward DNS lookup of the host and a reverse lookup of the IP address to obtain the hostname. If the reverse lookup fails the driver falls back to a cname lookup.falseor "none": Does no hostname canonicalization.When syncing the spec tests some other auth spec tests we hadn't synced yet were there, so I decided to leave them and skip the single failure.
This PR is branched off #3123 so needs to go in/be reviewed after that.
Is there new documentation needed for these changes?
None
What is the motivation for this change?
DRIVERS-1803 / NODE-2939
Double check the following
npm run check:lintscript<type>(NODE-xxxx)<!>: <description>