CDRIVER-598 check URI for bad readPreferenceTags

Author: ajdavis

NEWS

@@ -35,6 +35,7 @@ New features and bug fixes:
     exported symbols from the internal private ABI.
   * mongoc no longer crashes when multi roundtrip bulk operation fails.
   * Added support for the new readConcernLevel "linearizable".
+  * Clients now check for misformatted "readPreferenceTags" in URI.
   * New CMake option ENABLE_TRACING allows debug output, which before had only
     been available with "configure --enable-tracing".
   * Bugfix: "PossiblePrimary"-type replicas could be selected for reads

src/mongoc/mongoc-uri.c

@@ -462,7 +462,7 @@ mongoc_uri_parse_auth_mechanism_properties (mongoc_uri_t *uri,
    return true;
 }
 
-static void
+static bool
 mongoc_uri_parse_tags (mongoc_uri_t *uri, /* IN */
                        const char   *str) /* IN */
 {
@@ -476,22 +476,33 @@ mongoc_uri_parse_tags (mongoc_uri_t *uri, /* IN */
 
 again:
    if ((keyval = scan_to_unichar(str, ',', "", &end_keyval))) {
-      if ((key = scan_to_unichar(keyval, ':', "", &end_key))) {
-         bson_append_utf8(&b, key, -1, end_key + 1, -1);
-         bson_free(key);
+      if (!(key = scan_to_unichar(keyval, ':', "", &end_key))) {
+         bson_free (keyval);
+         goto fail;
       }
+
+      bson_append_utf8(&b, key, -1, end_key + 1, -1);
+      bson_free(key);
       bson_free(keyval);
       str = end_keyval + 1;
       goto again;
-   } else {
-       if ((key = scan_to_unichar(str, ':', "", &end_key))) {
-         bson_append_utf8(&b, key, -1, end_key + 1, -1);
-         bson_free(key);
-      }
+   } else if ((key = scan_to_unichar(str, ':', "", &end_key))) {
+      bson_append_utf8(&b, key, -1, end_key + 1, -1);
+      bson_free(key);
+   } else if (strlen (str)) {
+      /* we're not finished but we couldn't parse the string */
+      goto fail;
    }
 
    mongoc_read_prefs_add_tag(uri->read_prefs, &b);
    bson_destroy(&b);
+
+   return true;
+
+fail:
+   MONGOC_WARNING ("invalid readPreferenceTags: \"%s\"", str);
+   bson_destroy (&b);
+   return false;
 }
 
 
@@ -635,7 +646,9 @@ mongoc_uri_parse_option (mongoc_uri_t *uri,
                         (0 == strcasecmp (value, "t")) ||
                         (0 == strcmp (value, "1")));
    } else if (!strcasecmp(key, "readpreferencetags")) {
-      mongoc_uri_parse_tags(uri, value);
+      if (!mongoc_uri_parse_tags(uri, value)) {
+         goto CLEANUP;
+      }
    } else if (!strcasecmp(key, "authmechanism") ||
               !strcasecmp(key, "authsource")) {
       bson_append_utf8(&uri->credentials, key, -1, value, -1);

tests/test-mongoc-uri.c

@@ -604,6 +604,7 @@ typedef struct
    bool                parses;
    mongoc_read_mode_t  mode;
    bson_t             *tags;
+   const char         *log_msg;
 } read_prefs_test;
 
 
@@ -631,6 +632,8 @@ test_mongoc_uri_read_prefs (void)
       "0", "{", "}"
    );
 
+   const char *conflicts = "Primary read preference mode conflicts with tags";
+
    const read_prefs_test tests [] = {
       { "mongodb://localhost/", true, MONGOC_READ_PRIMARY, NULL },
       { "mongodb://localhost/?slaveOk=false", true, MONGOC_READ_PRIMARY, NULL },
@@ -643,13 +646,19 @@ test_mongoc_uri_read_prefs (void)
       /* readPreference should take priority over slaveOk */
       { "mongodb://localhost/?slaveOk=false&readPreference=secondary", true, MONGOC_READ_SECONDARY, NULL },
       /* readPreferenceTags conflict with primary mode */
-      { "mongodb://localhost/?readPreferenceTags=", false },
-      { "mongodb://localhost/?readPreference=primary&readPreferenceTags=", false },
-      { "mongodb://localhost/?slaveOk=false&readPreferenceTags=", false },
+      { "mongodb://localhost/?readPreferenceTags=", NULL, MONGOC_READ_PRIMARY, NULL, conflicts },
+      { "mongodb://localhost/?readPreference=primary&readPreferenceTags=", NULL, MONGOC_READ_PRIMARY, NULL, conflicts },
+      { "mongodb://localhost/?slaveOk=false&readPreferenceTags=", NULL, MONGOC_READ_PRIMARY, NULL, conflicts },
       { "mongodb://localhost/?readPreference=secondaryPreferred&readPreferenceTags=", true, MONGOC_READ_SECONDARY_PREFERRED, tags_empty },
       { "mongodb://localhost/?readPreference=secondaryPreferred&readPreferenceTags=dc:ny", true, MONGOC_READ_SECONDARY_PREFERRED, tags_dcny },
       { "mongodb://localhost/?readPreference=nearest&readPreferenceTags=dc:ny&readPreferenceTags=", true, MONGOC_READ_NEAREST, tags_dcny_empty },
       { "mongodb://localhost/?readPreference=nearest&readPreferenceTags=dc:ny,use:ssd&readPreferenceTags=dc:sf&readPreferenceTags=", true, MONGOC_READ_NEAREST, tags_dcnyusessd_dcsf_empty },
+      { "mongodb://localhost/?readPreference=nearest&readPreferenceTags=foo", false, MONGOC_READ_NEAREST, NULL,
+        "invalid readPreferenceTags: \"foo\""},
+      { "mongodb://localhost/?readPreference=nearest&readPreferenceTags=foo,bar", false, MONGOC_READ_NEAREST, NULL,
+        "invalid readPreferenceTags: \"foo,bar\""},
+      { "mongodb://localhost/?readPreference=nearest&readPreferenceTags=1", false, MONGOC_READ_NEAREST, NULL,
+        "invalid readPreferenceTags: \"1\""},
       { NULL }
    };
 
@@ -663,9 +672,10 @@ test_mongoc_uri_read_prefs (void)
          ASSERT_NO_CAPTURED_LOGS (t->uri);
       } else {
          assert(!uri);
-         ASSERT_CAPTURED_LOG (
-            t->uri, MONGOC_LOG_LEVEL_WARNING,
-            "Primary read preference mode conflicts with tags");
+         if (t->log_msg) {
+            ASSERT_CAPTURED_LOG (t->uri, MONGOC_LOG_LEVEL_WARNING, t->log_msg);
+         }
+
          continue;
       }