CSHARP-5200: Allow valid SRV hostnames with less than 3 parts #1527
Conversation
| var exception = Record.Exception(() => connectionString.Resolve()); | ||
|
|
||
| exception.Should().BeOfType<MongoConfigurationException>(). | ||
| Which.Message.Should().Be("Hosts in the SRV record must have the same parent domain as the seed host."); |
There was a problem hiding this comment.
How much specific should we be with the error message? If we wanted to be very precise it could become:
"Hosts in the SRV record must have the same parent domain as the seed host. If the seed host has less than three separated dot separated parts, then the returned hostname must have at least one more domain level than the seed".
Would it be too much?
There was a problem hiding this comment.
@sanych-sun What do you think?
There was a problem hiding this comment.
I'm OK with the current message.
...goDB.Driver.Tests/Specifications/initial-dns-seedlist-discovery/prose-tests/SrvPartsTests.cs
Show resolved
Hide resolved
| Func<string, string[]> getParentParts = x => x.Split('.').Skip(1).ToArray(); | ||
| var host = resolvedEndPoint.Host; | ||
|
|
||
| var hostParts = host.Split('.').ToArray(); |
There was a problem hiding this comment.
Can we avoid array allocation here? By counting number of '.' chars for length and checking if resolved host ends with original where we are comparing parts now.
There was a problem hiding this comment.
Makes sense! I've rewritten the method, hope it's readable enough
| for (int ai = a.Length - 1, bi = b.Length - 1; bi >= 0; ai--, bi--) | ||
| // We check that all the parts are equal, going from the end and arriving at the | ||
| // first dot of original (where the subdomain is, if we continued) | ||
| for (int hostIndex = host.Length - 1, originalIndex = original.Length - 1; |
There was a problem hiding this comment.
Can this loop also be replaced with EndsWith?
There was a problem hiding this comment.
You're right. I've rewritten the method to use EndsWith and use spans as much as possible to avoid string allocations
No description provided.