[CDRIVER-3723] Validate URI options on client/client pool creation Created: 18/Jun/20  Updated: 28/Oct/23  Resolved: 04/Jan/22

Status: Closed
Project: C Driver
Component/s: libmongoc
Affects Version/s: None
Fix Version/s: 1.21.0

Type: Improvement Priority: Major - P3
Reporter: Kevin Albertson Assignee: Jeremy Mikola
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Depends
is depended on by CDRIVER-4253 Use with_error client and pool constr... Backlog
is depended on by CDRIVER-4254 Prohibit construction of invalid topo... Backlog
is depended on by PHPC-1647 Defer to libmongoc for cross-option U... Closed
is depended on by CDRIVER-4255 Add assertions for non-null client an... Closed
Duplicate
is duplicated by CDRIVER-4254 Prohibit construction of invalid topo... Backlog
Related
related to CDRIVER-4251 mongoc_client_new_from_uri may leak t... Closed
related to CDRIVER-4250 Create public API to expose mongoc_ur... Closed
related to PHPC-1950 Lift restriction on authSource withou... Closed
is related to CDRIVER-3476 Lowercase options before storing when... Closed

 Description   

URI options may be passed to a mongoc_uri_t in three ways.

1. Through the string passed to the mongoc_uri_new call.

uri = mongoc_uri_new ("mongodb://localhost:27017/?ssl=true");

 
2. After creation, through the mongoc_uri_set_option_as_* helpers 

uri = mongoc_uri_new ("mongodb://localhost:27017/");
mongoc_uri_set_option_as_bool (uri, "ssl", false);

 
3. Via TXT records on a mongodb+srv URI.
 
mongoc_uri_parse_options validates that there are no conflicts between canonical and non-canonical values (e.g. having both ssl=true and tls=false). It is called for both (1) and (3).

mongoc_uri_finalize_tls, mongoc_uri_finalize_auth, mongoc_uri_finalize_directconnection validate other state of URI options. It is only called for (1).

Options set by (2) may not undergo validation unless they validated from options being applied by (3).

URI options should be validated regardless of how options are set. The proposed solution is to validate URI options on client/client pool construction by introducing the new APIs:

mongoc_client_t* mongoc_client_new_with_error (mongoc_uri_t* uri, bson_error_t* error);
mongoc_client_pool_t* mongoc_client_pool_new_with_error (mongoc_uri_t* uri, bson_error_t* error);



 Comments   
Comment by Githook User [ 04/Jan/22 ]

Author:

{'name': 'Jeremy Mikola', 'email': 'jmikola@gmail.com', 'username': 'jmikola'}

Message: CDRIVER-3723 finalize URI during client/pool construction (#921)

Introduces new mongoc_client_new_from_uri_with_error and mongoc_client_pool_new_with_error methods. Also addresses topology leak in mongoc_client_new_from_uri (CDRIVER-4251).

  • Remove redundant warnings about unused results
  • Explain why we need not handle null result from _mongoc_client_new_from_topology
  • Log URI errors in mongoc_client_new as warnings
  • Clear logs between /Client/max_staleness assertions
  • Set error instead of logging in mongoc_client_pool_new_with_error
  • Remove test expecting a client with an invalid topology
  • Revise assertions in /loadbalanced/client_uri_validation test

This no longer requires constructing a client and expecting an error through server selection.

  • Use tracing macro for return statement
  • Check MONGOC_ENABLE_SSL in mongoc_client_new_from_uri_with_error

Moving the check up from _mongoc_client_new_from_topology makes mongoc_client_new_from_uri_with_error consistent with mongoc_client_pool_new_with_error. Additionally, we add log message assertions to corresponding tests.

  • Anticipate null pool or client in SRV error tests
  • Remove obsolete tests for invalid LB topologies

These tests were previously introduced in CDRIVER-4184 but no longer apply now that we prohibit construction of clients and pools with an invalid topology.

  • _mongoc_client_new_from_topology should require a valid topology
  • Document precondition for _mongoc_client_new_from_topology

Co-authored-by: Ezra Chung <88335979+eramongodb@users.noreply.github.com>
Co-authored-by: Kevin Albertson <kevin.albertson@mongodb.com>
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/e7e15002d63cb57424f467c5f21eafa9ec0f018e

Comment by Jeremy Mikola [ 23/Dec/21 ]

https://github.com/mongodb/mongo-c-driver/pull/921

Comment by Kevin Albertson [ 02/Sep/20 ]

Hi pierremickael.gonzalo@gmail.com, with the new proposed APIs, the mongoc_uri_t is still created and parsed before the client. Avoiding double validation is a good point. Internally, the mongoc_uri_t could track whether options were set after the initial parsing to avoid an unnecessary revalidation when constructing the client.

uri = mongoc_uri_new_with_error ("mongodb://localhost:27017", &error);
if (!uri) { /* handle error */ }
mongoc_uri_set_option_as_bool (uri, "tls", true);
client = mongoc_client_new_with_error (uri, &error);
/* validation occurs again since set_option was called */

Setting options via TXT records is referencing the Initial DNS Seedlist Discovery specification. That sets additional options after a DNS lookup of TXT records, and has additional constraints.

I'm removing the new-eng label from this ticket. I think this may require some more involved restructuring of mongoc-uri.c parsing.

Comment by Pierre Mickael GONZALO [ 07/Aug/20 ]

Hi,

I have a question about the new prototype function of the two news api:

So the uri argument would be still not initialized when calling those apis ?

3. Via TXT records on a mongodb+srv URI.

you mean : initialize directly a mongoc_uri_t and set its attribute str ?

So the thing is to call "mongoc_uri_parse" is these new api to validate URI options ?
Then call mongoc_client_new/mongoc_client_pool_new ?
But not exactly because by calling mongoc_client_new, the uri options will be validated twice.

Generated at Wed Feb 07 21:18:51 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.