PHPC-1272: phongo_execute_query() should not alter state of Query opts

derickr · derickr · commit 3624e5acfd1d · 2018-09-20T17:37:09.000+01:00
diff --git a/php_phongo.c b/php_phongo.c
@@ -760,6 +760,7 @@ bool phongo_cursor_advance_and_check_for_error(mongoc_cursor_t* cursor TSRMLS_DC
 bool phongo_execute_query(mongoc_client_t* client, const char* namespace, zval* zquery, zval* options, uint32_t server_id, zval* return_value, int return_value_used TSRMLS_DC) /* {{{ */
 {
 	const php_phongo_query_t* query;
+	bson_t                    opts  = BSON_INITIALIZER;
 	mongoc_cursor_t*          cursor;
 	char*                     dbname;
 	char*                     collname;
@@ -777,30 +778,36 @@ bool phongo_execute_query(mongoc_client_t* client, const char* namespace, zval*
 
 	query = Z_QUERY_OBJ_P(zquery);
 
+	bson_copy_to(query->opts, &opts);
+
 	if (query->read_concern) {
 		mongoc_collection_set_read_concern(collection, query->read_concern);
 	}
 
 	if (!phongo_parse_read_preference(options, &zreadPreference TSRMLS_CC)) {
 		/* Exception should already have been thrown */
 		mongoc_collection_destroy(collection);
+		bson_destroy(&opts);
 		return false;
 	}
 
-	if (!phongo_parse_session(options, client, query->opts, &zsession TSRMLS_CC)) {
+	if (!phongo_parse_session(options, client, &opts, &zsession TSRMLS_CC)) {
 		/* Exception should already have been thrown */
 		mongoc_collection_destroy(collection);
+		bson_destroy(&opts);
 		return false;
 	}
 
-	if (!BSON_APPEND_INT32(query->opts, "serverId", server_id)) {
+	if (!BSON_APPEND_INT32(&opts, "serverId", server_id)) {
 		phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT TSRMLS_CC, "Error appending \"serverId\" option");
 		mongoc_collection_destroy(collection);
+		bson_destroy(&opts);
 		return false;
 	}
 
-	cursor = mongoc_collection_find_with_opts(collection, query->filter, query->opts, phongo_read_preference_from_zval(zreadPreference TSRMLS_CC));
+	cursor = mongoc_collection_find_with_opts(collection, query->filter, &opts, phongo_read_preference_from_zval(zreadPreference TSRMLS_CC));
 	mongoc_collection_destroy(collection);
+	bson_destroy(&opts);
 
 	/* maxAwaitTimeMS must be set before the cursor is sent */
 	if (query->max_await_time_ms) {
diff --git a/tests/manager/manager-executeQuery-001.phpt b/tests/manager/manager-executeQuery-001.phpt
@@ -56,8 +56,6 @@ object(MongoDB\Driver\Cursor)#%d (%d) {
         ["y"]=>
         int(1)
       }
-      ["serverId"]=>
-      int(%d)
     }
     ["readConcern"]=>
     NULL
diff --git a/tests/manager/manager-executeQuery-002.phpt b/tests/manager/manager-executeQuery-002.phpt
@@ -55,8 +55,6 @@ object(MongoDB\Driver\Cursor)#%d (%d) {
         ["y"]=>
         int(1)
       }
-      ["serverId"]=>
-      int(%d)
     }
     ["readConcern"]=>
     NULL
diff --git a/tests/readPreference/bug0146-001.phpt b/tests/readPreference/bug0146-001.phpt
@@ -46,8 +46,6 @@ object(MongoDB\Driver\Cursor)#%d (%d) {
     }
     ["options"]=>
     object(stdClass)#%d (%d) {
-      ["serverId"]=>
-      int(%d)
     }
     ["readConcern"]=>
     NULL
@@ -86,8 +84,6 @@ object(MongoDB\Driver\Cursor)#%d (%d) {
     }
     ["options"]=>
     object(stdClass)#%d (%d) {
-      ["serverId"]=>
-      int(%d)
     }
     ["readConcern"]=>
     NULL
@@ -126,8 +122,6 @@ object(MongoDB\Driver\Cursor)#%d (%d) {
     }
     ["options"]=>
     object(stdClass)#%d (%d) {
-      ["serverId"]=>
-      int(%d)
     }
     ["readConcern"]=>
     NULL
@@ -166,8 +160,6 @@ object(MongoDB\Driver\Cursor)#%d (%d) {
     }
     ["options"]=>
     object(stdClass)#%d (%d) {
-      ["serverId"]=>
-      int(%d)
     }
     ["readConcern"]=>
     NULL
@@ -206,8 +198,6 @@ object(MongoDB\Driver\Cursor)#%d (%d) {
     }
     ["options"]=>
     object(stdClass)#%d (%d) {
-      ["serverId"]=>
-      int(%d)
     }
     ["readConcern"]=>
     NULL
diff --git a/tests/readPreference/bug0146-002.phpt b/tests/readPreference/bug0146-002.phpt
@@ -46,8 +46,6 @@ object(MongoDB\Driver\Cursor)#%d (%d) {
     }
     ["options"]=>
     object(stdClass)#%d (%d) {
-      ["serverId"]=>
-      int(%d)
     }
     ["readConcern"]=>
     NULL
@@ -86,8 +84,6 @@ object(MongoDB\Driver\Cursor)#%d (%d) {
     }
     ["options"]=>
     object(stdClass)#%d (%d) {
-      ["serverId"]=>
-      int(%d)
     }
     ["readConcern"]=>
     NULL
@@ -126,8 +122,6 @@ object(MongoDB\Driver\Cursor)#%d (%d) {
     }
     ["options"]=>
     object(stdClass)#%d (%d) {
-      ["serverId"]=>
-      int(%d)
     }
     ["readConcern"]=>
     NULL
@@ -166,8 +160,6 @@ object(MongoDB\Driver\Cursor)#%d (%d) {
     }
     ["options"]=>
     object(stdClass)#%d (%d) {
-      ["serverId"]=>
-      int(%d)
     }
     ["readConcern"]=>
     NULL
@@ -206,8 +198,6 @@ object(MongoDB\Driver\Cursor)#%d (%d) {
     }
     ["options"]=>
     object(stdClass)#%d (%d) {
-      ["serverId"]=>
-      int(%d)
     }
     ["readConcern"]=>
     NULL

Original file line number	Diff line number	Diff line change
`@@ -56,8 +56,6 @@ object(MongoDB\Driver\Cursor)#%d (%d) {`
`56`	`56`	`["y"]=>`
`57`	`57`	`int(1)`
`58`	`58`	`}`
`59`		`- ["serverId"]=>`
`60`		`- int(%d)`
`61`	`59`	`}`
`62`	`60`	`["readConcern"]=>`
`63`	`61`	`NULL`
Original file line number	Diff line number	Diff line change
`@@ -55,8 +55,6 @@ object(MongoDB\Driver\Cursor)#%d (%d) {`
`55`	`55`	`["y"]=>`
`56`	`56`	`int(1)`
`57`	`57`	`}`
`58`		`- ["serverId"]=>`
`59`		`- int(%d)`
`60`	`58`	`}`
`61`	`59`	`["readConcern"]=>`
`62`	`60`	`NULL`
Original file line number	Diff line number	Diff line change
`@@ -46,8 +46,6 @@ object(MongoDB\Driver\Cursor)#%d (%d) {`
`46`	`46`	`}`
`47`	`47`	`["options"]=>`
`48`	`48`	`object(stdClass)#%d (%d) {`
`49`		`- ["serverId"]=>`
`50`		`- int(%d)`
`51`	`49`	`}`
`52`	`50`	`["readConcern"]=>`
`53`	`51`	`NULL`
`@@ -86,8 +84,6 @@ object(MongoDB\Driver\Cursor)#%d (%d) {`
`86`	`84`	`}`
`87`	`85`	`["options"]=>`
`88`	`86`	`object(stdClass)#%d (%d) {`
`89`		`- ["serverId"]=>`
`90`		`- int(%d)`
`91`	`87`	`}`
`92`	`88`	`["readConcern"]=>`
`93`	`89`	`NULL`
`@@ -126,8 +122,6 @@ object(MongoDB\Driver\Cursor)#%d (%d) {`
`126`	`122`	`}`
`127`	`123`	`["options"]=>`
`128`	`124`	`object(stdClass)#%d (%d) {`
`129`		`- ["serverId"]=>`
`130`		`- int(%d)`
`131`	`125`	`}`
`132`	`126`	`["readConcern"]=>`
`133`	`127`	`NULL`
`@@ -166,8 +160,6 @@ object(MongoDB\Driver\Cursor)#%d (%d) {`
`166`	`160`	`}`
`167`	`161`	`["options"]=>`
`168`	`162`	`object(stdClass)#%d (%d) {`
`169`		`- ["serverId"]=>`
`170`		`- int(%d)`
`171`	`163`	`}`
`172`	`164`	`["readConcern"]=>`
`173`	`165`	`NULL`
`@@ -206,8 +198,6 @@ object(MongoDB\Driver\Cursor)#%d (%d) {`
`206`	`198`	`}`
`207`	`199`	`["options"]=>`
`208`	`200`	`object(stdClass)#%d (%d) {`
`209`		`- ["serverId"]=>`
`210`		`- int(%d)`
`211`	`201`	`}`
`212`	`202`	`["readConcern"]=>`
`213`	`203`	`NULL`