Uploaded image for project: 'Compass '
  1. Compass
  2. COMPASS-6717

Investigate changes in NODE-5042: Relax SRV record validation to account for a "." suffix

    • Type: Icon: Investigation Investigation
    • Resolution: Unresolved
    • Priority: Icon: Major - P3 Major - P3
    • None
    • Affects Version/s: None
    • Component/s: None
    • 2

      How are you using Mongo? What version of the server and driver are you using?

      5.0 driver code

      What is the feature/improvement you would like?

      SRV record validation should account for possible trailing "." on SRV records without throwing an error.  For example, the SRV host "freecluster-oztdp.mongodb-dev.net" could resolve to SRV records like:

      freecluster-shard-00-00-oztdp.mongodb-dev.net.
      freecluster-shard-00-01-oztdp.mongodb-dev.net.
      freecluster-shard-00-02-oztdp.mongodb-dev.net.
      

      and these should not fail validation in the matchesParentDomain function. 

      What use case would this feature/improvement enable?

      I suspect this would fix (or work around, depending on your perspective)  the issue reported about Deno by alex.bevilacqua@mongodb.com here: https://github.com/denoland/deno/issues/16633. To confirm this I ran the following script, which is just stripping out everything but the SRV lookup logic from the driver's connection_string.ts :

      Unable to find source-code formatter for language: ts. Available languages are: actionscript, ada, applescript, bash, c, c#, c++, cpp, css, erlang, go, groovy, haskell, html, java, javascript, js, json, lua, none, nyan, objc, perl, php, python, r, rainbow, ruby, scala, sh, sql, swift, visualbasic, xml, yaml
      import * as dns from "node:dns";
      
      function matchesParentDomain(srvAddress: string, parentDomain: string): boolean {
        const regex = /^.*?\./;
        const srv = `.${srvAddress.replace(regex, '')}`;
        const parent = `.${parentDomain.replace(regex, '')}`;
        return srv.endsWith(parent);
      }
      
      async function main() {
        const lookupAddress = 'freecluster-oztdp.mongodb-dev.net';
        const srvServiceName = 'mongodb';
        const addresses = await dns.promises.resolveSrv(
          `_${srvServiceName}._tcp.${lookupAddress}`
        );
      
        console.log('%s', lookupAddress);
        console.log('%s', srvServiceName);
      
        for (const { name } of addresses) {
          console.log('%s', name);
          if (!matchesParentDomain(name, lookupAddress)) {
            console.log('Name does not match parent domain');
          }
        }
      }
      
      main().then(r => console.log('Done')) 
      

      WIth npx, the output is

      % npx ts-node srv_test.ts
      freecluster-oztdp.mongodb-dev.net
      mongodb
      freecluster-shard-00-00-oztdp.mongodb-dev.net
      freecluster-shard-00-01-oztdp.mongodb-dev.net
      freecluster-shard-00-02-oztdp.mongodb-dev.net
      Done
      

      while with deno it's:

      % deno run --unstable --allow-all srv_test.ts
      freecluster-oztdp.mongodb-dev.net
      mongodb
      freecluster-shard-00-00-oztdp.mongodb-dev.net.
      Name does not match parent domain
      freecluster-shard-00-01-oztdp.mongodb-dev.net.
      Name does not match parent domain
      freecluster-shard-00-02-oztdp.mongodb-dev.net.
      Name does not match parent domain
      Done
      

      Note that the "raw" SRV record does contain the trailing ".", but it seems like Node strips that "." from the response, while deno does not.

      I also notice that in the Java driver the following check exists:

      // The "raw" srvRecord looks like: 
      // "0 0 27017 freecluster-shard-00-00-oztdp.mongodb-dev.net."
      // coming from Java's DNS lookup library.  Note the trailing "."
      String[] split = srvRecord.split(" ");  
      String resolvedHost = split[3].endsWith(".")        
            ? split[3].substring(0, split[3].length() - 1)        
            : split[3];
      

      So it's accounting for the possibility of a trailing ".", but does not assume it will be there. I don't recall why it was done this way, and I don't see anything in the spec to account for the conditional logic.

            Assignee:
            Unassigned Unassigned
            Reporter:
            dbeng-pm-bot PM Bot
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

              Created:
              Updated: