[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:
Depends
depends on DRIVERS-2655 Include Connection URI Options as Han... Closed
Duplicate
Issue split
split to CDRIVER-4558 Require Drivers to raise errors for i... Closed
split to CSHARP-4484 Require Drivers to raise errors for i... Closed
split to CXX-2639 Require Drivers to raise errors for i... Closed
split to GODRIVER-2738 Require Drivers to raise errors for i... Closed
split to JAVA-4847 Require Drivers to raise errors for i... Closed
split to MOTOR-1085 Require Drivers to raise errors for i... Closed
split to NODE-4972 Require Drivers to raise errors for i... Closed
split to PHPLIB-1067 Require Drivers to raise errors for i... Closed
split to PYTHON-3572 Require Drivers to raise errors for i... Closed
split to RUBY-3207 Require Drivers to raise errors for i... Closed
split to RUST-1572 Require Drivers to raise errors for i... Closed
Related
related to CDRIVER-3167 URI Spec conformance Backlog
is related to CDRIVER-2869 Implement Unified URI Options Closed
Engineering Lead: Daria Pardue Daria Pardue
Product Manager: Alex Bevilacqua Alex Bevilacqua
Program Manager: Jessica Sigafoos Jessica Sigafoos
Start date:
Scope Cost Estimate: 0
Cost to Date: 0
Final Cost Estimate: 0
Cost Threshold %: 100
Driver Compliance:
Key Status/Resolution FixVersion
CDRIVER-4558 Won't Fix
CXX-2639 Won't Fix
CSHARP-4484 Won't Fix
GODRIVER-2738 Won't Fix 2.0.0
JAVA-4847 Won't Fix
NODE-4972 Won't Fix
MOTOR-1085 Won't Fix
PYTHON-3572 Won't Fix
PHPLIB-1067 Won't Fix
RUBY-3207 Won't Fix
RUST-1572 Won't Fix
SWIFT-1693 Won't Do

 Description   

While sara.golemon was implementing CDRIVER-2869. she noticed that the "Valid required tls options are parsed correctly" test in tls-options.json failed because the spec expects a valid URI string and a logged warning. That agrees with the last paragraph in Connection String Spec: Values:

Any invalid Values for a given key MUST be ignored and MUST log a WARN level message)

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:

valid:: a boolean indicating if the write concern created from the document is valid.

and

Testing whether a URI is valid or not should simply be a matter of checking whether URI parsing raises an error or exception.

and has a case like this:

 description: "wtimeoutMS as an invalid number"
        uri: "mongodb://localhost/?wtimeoutMS=-500"
        valid: false
        warning: ~

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:

I'm re-doing the Perl URI parsing to account for tests since the initial implementation and have been wondering the same thing.

Another one I found annoying is mongodb://example.com/?readPreferenceTags=invalid because "invalid" isn't in the "x:y" form that would represent a valid tag. Is that a structural problem that should error? Or an invalid value that should merely warn. Test says valid, but warn.

The connection string spec is ambiguous whether an "invalid value" mean invalid type or invalid data or both.

Generated at Thu Feb 08 08:24:36 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.