Skip to content

fix(NODE-5042): relax SRV record validation to account for a dot suffix #3633

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 4 commits into from
Apr 12, 2023

Conversation

nbbeeken
Copy link
Contributor

@nbbeeken nbbeeken commented Apr 10, 2023

Description

What is changing?

Allows for SRV record addresses to have a trailing dot

Verified manually that this makes deno connect successfully with an SRV record.

To reproduce my success: 😎

  • deno init
  • Add a package.json with mongodb as a dependency
  • import { MongoClient } from 'mongodb'
  • deno run --inspect-brk --allow-read --allow-sys --allow-env --allow-net main.ts
  • I npm pack-ed this branch and replaced the folder here: node_modules/.deno/mongodb@5.2.0/node_modules/mongodb
  • use the deno run command again above and successfully connect to Atlas

What is the motivation for this change?

A trailing dot on a hostname does not prevent dns.lookup resolution so our validation can be made more lenient.

Double check the following

  • Ran npm run check:lint script
  • Self-review completed using the steps outlined here
  • PR title follows the correct format: type(NODE-xxxx)[!]: description
    • Example: feat(NODE-1234)!: rewriting everything in coffeescript
  • Changes are covered by tests
  • New TODOs have a related JIRA ticket

@nbbeeken nbbeeken force-pushed the NODE-5042-srv-dot branch from eb8e384 to 70ce937 Compare April 10, 2023 20:52
@nbbeeken nbbeeken marked this pull request as ready for review April 10, 2023 20:57
@durran durran self-assigned this Apr 11, 2023
@durran durran added the Primary Review In Review with primary reviewer, not yet ready for team's eyes label Apr 11, 2023
@nbbeeken nbbeeken requested a review from durran April 11, 2023 18:00
Copy link
Contributor

@addaleax addaleax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just hitting the red X because of the comment above, feel free to dismiss

@nbbeeken nbbeeken requested a review from addaleax April 11, 2023 19:12
@addaleax addaleax dismissed their stale review April 11, 2023 20:31

addressed

@durran durran added Team Review Needs review from team and removed Primary Review In Review with primary reviewer, not yet ready for team's eyes labels Apr 12, 2023
@durran durran removed their assignment Apr 12, 2023
@durran durran merged commit ad15881 into main Apr 12, 2023
@durran durran deleted the NODE-5042-srv-dot branch April 12, 2023 11:02
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Team Review Needs review from team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants