PHPC-1216: Always validate and apply wtimeoutMS in URI options array

Author: jmikola

php_phongo.c

@@ -1788,7 +1788,6 @@ static bool php_phongo_apply_rp_options_to_uri(mongoc_uri_t* uri, bson_t* option
 static bool php_phongo_apply_wc_options_to_uri(mongoc_uri_t* uri, bson_t* options TSRMLS_DC) /* {{{ */
 {
 	bson_iter_t                   iter;
-	int32_t                       wtimeoutms;
 	mongoc_write_concern_t*       new_wc;
 	const mongoc_write_concern_t* old_wc;
 
@@ -1811,8 +1810,6 @@ static bool php_phongo_apply_wc_options_to_uri(mongoc_uri_t* uri, bson_t* option
 		return true;
 	}
 
-	wtimeoutms = mongoc_write_concern_get_wtimeout(old_wc);
-
 	new_wc = mongoc_write_concern_copy(old_wc);
 
 	if (bson_iter_init_find_case(&iter, options, MONGOC_URI_SAFE)) {
@@ -1827,14 +1824,25 @@ static bool php_phongo_apply_wc_options_to_uri(mongoc_uri_t* uri, bson_t* option
 	}
 
 	if (bson_iter_init_find_case(&iter, options, MONGOC_URI_WTIMEOUTMS)) {
+		int32_t wtimeout;
+
 		if (!BSON_ITER_HOLDS_INT32(&iter)) {
 			PHONGO_URI_INVALID_TYPE(iter, "32-bit integer");
 			mongoc_write_concern_destroy(new_wc);
 
 			return false;
 		}
 
-		wtimeoutms = bson_iter_int32(&iter);
+		wtimeout = bson_iter_int32(&iter);
+
+		if (wtimeout < 0) {
+			phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT TSRMLS_CC, "Expected wtimeoutMS to be >= 0, %d given", wtimeout);
+			mongoc_write_concern_destroy(new_wc);
+
+			return false;
+		}
+
+		mongoc_write_concern_set_wtimeout(new_wc, wtimeout);
 	}
 
 	if (bson_iter_init_find_case(&iter, options, MONGOC_URI_JOURNAL)) {
@@ -1872,7 +1880,8 @@ static bool php_phongo_apply_wc_options_to_uri(mongoc_uri_t* uri, bson_t* option
 			const char* str = bson_iter_utf8(&iter, NULL);
 
 			if (0 == strcasecmp(PHONGO_WRITE_CONCERN_W_MAJORITY, str)) {
-				mongoc_write_concern_set_wmajority(new_wc, wtimeoutms);
+				/* wtimeoutMS is set independently, so preserve its value here */
+				mongoc_write_concern_set_wmajority(new_wc, mongoc_write_concern_get_wtimeout(new_wc));
 			} else {
 				mongoc_write_concern_set_wtag(new_wc, str);
 			}
@@ -1884,15 +1893,6 @@ static bool php_phongo_apply_wc_options_to_uri(mongoc_uri_t* uri, bson_t* option
 		}
 	}
 
-	/* Only set wtimeout if it's still applicable; otherwise, clear it. */
-	if (mongoc_write_concern_get_w(new_wc) > 1 ||
-		mongoc_write_concern_get_wmajority(new_wc) ||
-		mongoc_write_concern_get_wtag(new_wc)) {
-		mongoc_write_concern_set_wtimeout(new_wc, wtimeoutms);
-	} else {
-		mongoc_write_concern_set_wtimeout(new_wc, 0);
-	}
-
 	if (mongoc_write_concern_get_journal(new_wc)) {
 		int32_t w = mongoc_write_concern_get_w(new_wc);
 

tests/manager/manager-ctor-write_concern-002.phpt

@@ -4,7 +4,6 @@ MongoDB\Driver\Manager::__construct(): write concern options (wtimeoutms)
 <?php
 
 $tests = [
-    // wtimeoutms does not get applied unless w > 1, w = majority, or tag sets are used
     ['mongodb://127.0.0.1/?wtimeoutms=1000', []],
     ['mongodb://127.0.0.1/?w=2&wtimeoutms=1000', []],
     ['mongodb://127.0.0.1/?w=majority&wtimeoutms=1000', []],
@@ -27,6 +26,8 @@ foreach ($tests as $test) {
 <?php exit(0); ?>
 --EXPECTF--
 object(MongoDB\Driver\WriteConcern)#%d (%d) {
+  ["wtimeout"]=>
+  int(1000)
 }
 object(MongoDB\Driver\WriteConcern)#%d (%d) {
   ["w"]=>
@@ -47,6 +48,8 @@ object(MongoDB\Driver\WriteConcern)#%d (%d) {
   int(1000)
 }
 object(MongoDB\Driver\WriteConcern)#%d (%d) {
+  ["wtimeout"]=>
+  int(1000)
 }
 object(MongoDB\Driver\WriteConcern)#%d (%d) {
   ["w"]=>

tests/manager/manager-ctor-write_concern-error-004.phpt

@@ -1,14 +1,14 @@
 --TEST--
-MongoDB\Driver\Manager::__construct(): invalid write concern (wtimeoutms range)
+MongoDB\Driver\Manager::__construct(): invalid write concern (wtimeoutms range exceeds INT32_MAX)
 --SKIPIF--
 <?php if (8 !== PHP_INT_SIZE) { die('skip Only for 64-bit platform'); } ?>
 --FILE--
 <?php
 
 require_once __DIR__ . '/../utils/tools.php';
 
-/* Note: libmongoc does not check wTimeoutMS's range in the URI string. 64-bit
- * integers will be truncated by strtol() */
+/* Note: libmongoc does not check wTimeoutMS's upper bounds in the URI string.
+ * 64-bit integers will be truncated by strtol(). */
 
 echo throws(function() {
     new MongoDB\Driver\Manager(null, ['wTimeoutMS' => 2147483648]);

tests/manager/manager-ctor-write_concern-error-007.phpt

@@ -0,0 +1,20 @@
+--TEST--
+MongoDB\Driver\Manager::__construct(): invalid write concern (wtimeoutms range)
+--SKIPIF--
+<?php if (8 !== PHP_INT_SIZE) { die('skip Only for 64-bit platform'); } ?>
+--FILE--
+<?php
+
+require_once __DIR__ . '/../utils/tools.php';
+
+echo throws(function() {
+    new MongoDB\Driver\Manager(null, ['wTimeoutMS' => -1]);
+}, "MongoDB\Driver\Exception\InvalidArgumentException"), "\n";
+
+?>
+===DONE===
+<?php exit(0); ?>
+--EXPECT--
+OK: Got MongoDB\Driver\Exception\InvalidArgumentException
+Expected wtimeoutMS to be >= 0, -1 given
+===DONE===

tests/manager/manager-getwriteconcern-001.phpt

@@ -10,7 +10,6 @@ $tests = array(
     array(null, array('w' => 1, 'journal' => true)),
     array(null, array('w' => 'majority', 'journal' => true)),
     array('mongodb://127.0.0.1/?w=majority&journal=true', array('w' => 1, 'journal' => false)),
-    // wtimeoutms does not get applied unless w > 1, w = majority, or tag sets are used
     array('mongodb://127.0.0.1/?wtimeoutms=1000', array()),
     array(null, array('wtimeoutms' => 1000)),
     array('mongodb://127.0.0.1/?w=2', array('wtimeoutms' => 1000)),
@@ -61,8 +60,12 @@ object(MongoDB\Driver\WriteConcern)#%d (%d) {
   bool(false)
 }
 object(MongoDB\Driver\WriteConcern)#%d (%d) {
+  ["wtimeout"]=>
+  int(1000)
 }
 object(MongoDB\Driver\WriteConcern)#%d (%d) {
+  ["wtimeout"]=>
+  int(1000)
 }
 object(MongoDB\Driver\WriteConcern)#%d (%d) {
   ["w"]=>