[DRIVERS-2056] Require Drivers to raise errors for invalid URI Option Keys and Values Created: 29/May/19 Updated: 27/Jul/23 |
|
| Status: | Backlog |
| Project: | Drivers |
| Component/s: | Connection String, URI Options |
| Fix Version/s: | None |
| Type: | Epic | Priority: | Major - P3 |
| Reporter: | Jeremy Mikola | Assignee: | Unassigned |
| Resolution: | Unresolved | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Engineering Lead: | |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Product Manager: | |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Program Manager: | |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Start date: | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Scope Cost Estimate: | 0 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Cost to Date: | 0 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Final Cost Estimate: | 0 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Cost Threshold %: | 100 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Driver Compliance: |
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Description |
|
While sara.golemon was implementing
That said, I'm curious why we don't allow hard errors on such cases. The current spec behavior of logging a warning and ignoring the value could make it quite easy for users to miss the problem. Elsewhere in the connection string spec, warnings are suggested for using deprecated options (e.g. "yes" string for a boolean), which makes more sense as we're still applying the value and not ignoring it. In that case, there's no harm if the user misses a warning (hopefully they see the warning and fix the URI); however, if support for a deprecated option/value was ultimately dropped the driver would still only log a warning despite a likely behavioral change for the user's application. I'd propose that we at least allow drivers to raise an error for invalid (not merely deprecated) values. This would require some care when updating the tests, since we wouldn't want to simply remove existing test cases with invalid values. |
| Comments |
| Comment by PM Bot [ 18/May/23 ] | ||||
|
Alex Bevilacqua renamed project from "Require drivers to raise errors for invalid URI option values" to "Require Drivers to raise errors for invalid URI Option Keys and Values" | ||||
| Comment by David Golden [ 29/May/19 ] | ||||
|
The connection string and URI options specs are also inconsistent with the Read/Write concern spec tests, which say:
and
and has a case like this:
Per the connection string rules, an invalid (negative) wtimeeoutMS should not throw an exception. I'm curious how the drivers that have implemented URI options already have addressed this inconsistency. | ||||
| Comment by David Golden [ 29/May/19 ] | ||||
|
Semi-related, from a bug I found in my work to match the spec behavior: we appear to have no test coverage for `mongodb://localhost/?readPreference=primaryPreferred&readPreferenceTags=dc:ny,rack:1&readPreferenceTags=dc:ny&readPreferenceTags=invalid`, which per the spec should only ignore the last readPreferenceTags (and not all, as my buggy code did). I presume that also means we have no coverage for readPreferenceTags=dc:ny,invalid&readPreferenceTags=dc:va – which I think should ignore the first readPreferenceTags completely and not record it as "dc:ny". (The original Perl driver code would have thrown an error for any of these invalid cases.) | ||||
| Comment by David Golden [ 29/May/19 ] | ||||
|
If I understand correctly, there is also a security consideration: `mongodb://localhost/?tls=1.2`. Per the spec, that would warn but proceed without TLS, because the invalid tls value results in tls being ignored. Another example: `mongodb://localhost/?tls=&wtimeout=1000`. Is that a bad parse because tls didn't have a value? Is it parsed as the empty string? If empty string, that would also warn and ignore tls. | ||||
| Comment by Jeremy Mikola [ 29/May/19 ] | ||||
|
david.golden provided some more context in Slack:
|