Skip to content

Commit cceeda8

Browse files
jmikolajmikola
committed
PHPC-1163: Prohibit session with unacknowledged write concern
This adds checks to ensure that explicit sessions and unacknowledged write concerns (explicit or inherited) will not be mixed when executing bulk writes or commands. Additionally, we ensure that command execution does not create its own implicit session (per PHPC-1152) if the effective write concern is unacknowledged. Note: libmongoc still needs to ensure that it does not create implicit sessions for unacknowledged commands (CDRIVER-2615).
1 parent 2097bf3 commit cceeda8

6 files changed

+297
-15
lines changed

‎php_phongo.c

Lines changed: 41 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -673,6 +673,17 @@ bool phongo_execute_bulk_write(mongoc_client_t *client, const char *namespace, p
673673
return false;
674674
}
675675

676+
/* If a write concern was not specified, libmongoc will use the client's
677+
* write concern; however, we should still fetch it for the write result.
678+
* Additionally, we need to check if an unacknowledged write concern would
679+
* conflict with an explicit session. */
680+
write_concern = zwriteConcern ? Z_WRITECONCERN_OBJ_P(zwriteConcern)->write_concern : mongoc_client_get_write_concern(client);
681+
682+
if (zsession && !mongoc_write_concern_is_acknowledged(write_concern)) {
683+
phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT TSRMLS_CC, "Cannot combine \"session\" option with an unacknowledged write concern");
684+
return false;
685+
}
686+
676687
mongoc_bulk_operation_set_database(bulk, bulk_write->database);
677688
mongoc_bulk_operation_set_collection(bulk, bulk_write->collection);
678689
mongoc_bulk_operation_set_client(bulk, client);
@@ -682,13 +693,8 @@ bool phongo_execute_bulk_write(mongoc_client_t *client, const char *namespace, p
682693
mongoc_bulk_operation_set_client_session(bulk, Z_SESSION_OBJ_P(zsession)->client_session);
683694
}
684695

685-
/* If a write concern was not specified, libmongoc will use the client's
686-
* write concern; however, we should still fetch it for the write result. */
687-
write_concern = phongo_write_concern_from_zval(zwriteConcern TSRMLS_CC);
688-
if (write_concern) {
689-
mongoc_bulk_operation_set_write_concern(bulk, write_concern);
690-
} else {
691-
write_concern = mongoc_client_get_write_concern(client);
696+
if (zwriteConcern) {
697+
mongoc_bulk_operation_set_write_concern(bulk, Z_WRITECONCERN_OBJ_P(zwriteConcern)->write_concern);
692698
}
693699

694700
success = mongoc_bulk_operation_execute(bulk, &reply, &error);
@@ -862,6 +868,7 @@ bool phongo_execute_command(mongoc_client_t *client, php_phongo_command_type_t t
862868
bool result = false;
863869
bool free_reply = false;
864870
bool free_zsession = false;
871+
bool is_unacknowledged_write_concern = false;
865872

866873
command = Z_COMMAND_OBJ_P(zcommand);
867874

@@ -880,9 +887,33 @@ bool phongo_execute_command(mongoc_client_t *client, php_phongo_command_type_t t
880887
goto cleanup;
881888
}
882889

883-
/* If an explicit session was not provided, attempt to create an implicit
884-
* client session (ignoring any errors). */
885-
if (!zsession) {
890+
if (type & PHONGO_OPTION_WRITE_CONCERN) {
891+
zval *zwriteConcern = NULL;
892+
893+
if (!phongo_parse_write_concern(options, &opts, &zwriteConcern TSRMLS_CC)) {
894+
/* Exception should already have been thrown */
895+
goto cleanup;
896+
}
897+
898+
/* Determine if the explicit or inherited write concern is
899+
* unacknowledged so that we can ensure it does not conflict with an
900+
* explicit or implicit session. */
901+
if (zwriteConcern) {
902+
is_unacknowledged_write_concern = !mongoc_write_concern_is_acknowledged(Z_WRITECONCERN_OBJ_P(zwriteConcern)->write_concern);
903+
} else if (type != PHONGO_COMMAND_RAW) {
904+
is_unacknowledged_write_concern = !mongoc_write_concern_is_acknowledged(mongoc_client_get_write_concern(client));
905+
}
906+
}
907+
908+
if (zsession && is_unacknowledged_write_concern) {
909+
phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT TSRMLS_CC, "Cannot combine \"session\" option with an unacknowledged write concern");
910+
goto cleanup;
911+
}
912+
913+
/* If an explicit session was not provided and the effective write concern
914+
* is not unacknowledged, attempt to create an implicit client session
915+
* (ignoring any errors). */
916+
if (!zsession && !is_unacknowledged_write_concern) {
886917
zsession = phongo_create_implicit_session(client TSRMLS_CC);
887918

888919
if (zsession) {
@@ -895,11 +926,6 @@ bool phongo_execute_command(mongoc_client_t *client, php_phongo_command_type_t t
895926
}
896927
}
897928

898-
if ((type & PHONGO_OPTION_WRITE_CONCERN) && !phongo_parse_write_concern(options, &opts, NULL TSRMLS_CC)) {
899-
/* Exception should already have been thrown */
900-
goto cleanup;
901-
}
902-
903929
if (!BSON_APPEND_INT32(&opts, "serverId", server_id)) {
904930
phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT TSRMLS_CC, "Error appending \"serverId\" option");
905931
goto cleanup;

‎tests/manager/bug1163-001.phpt

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
--TEST--
2+
PHPC-1163: Unacknowledged write concern should omit implicit session
3+
--XFAIL--
4+
Depends on CDRIVER-2615
5+
--SKIPIF--
6+
<?php if (PHP_INT_SIZE !== 8) { die("skip Can't represent 64-bit ints on a 32-bit platform"); } ?>
7+
<?php require __DIR__ . "/../utils/basic-skipif.inc"; ?>
8+
<?php NEEDS('STANDALONE'); ?>
9+
<?php CLEANUP(STANDALONE); ?>
10+
--FILE--
11+
<?php
12+
require_once __DIR__ . "/../utils/basic.inc";
13+
14+
class Test implements MongoDB\Driver\Monitoring\CommandSubscriber
15+
{
16+
public function run()
17+
{
18+
$manager = new MongoDB\Driver\Manager(STANDALONE, ['w' => 0]);
19+
20+
MongoDB\Driver\Monitoring\addSubscriber($this);
21+
22+
$bulk = new MongoDB\Driver\BulkWrite;
23+
$bulk->insert(['x' => 1]);
24+
25+
echo "Testing executeBulkWrite\n";
26+
$manager->executeBulkWrite(NS, $bulk);
27+
28+
$command = new MongoDB\Driver\Command([
29+
'insert' => COLLECTION_NAME,
30+
'documents' => [['x' => 1]],
31+
]);
32+
33+
/* Note: executeCommand() and executeReadCommand() are not tested
34+
* because they do not inherit the client-level write concern. */
35+
36+
echo "\nTesting executeWriteCommand\n";
37+
$manager->executeWriteCommand(DATABASE_NAME, $command);
38+
39+
/* We can safely re-use the insert command with executeReadWriteCommand
40+
* because there is no readConcern to inherit. */
41+
echo "\nTesting executeReadWriteCommand\n";
42+
$manager->executeReadWriteCommand(DATABASE_NAME, $command);
43+
44+
MongoDB\Driver\Monitoring\removeSubscriber($this);
45+
}
46+
47+
public function commandStarted(MongoDB\Driver\Monitoring\CommandStartedEvent $event)
48+
{
49+
if ($event->getCommandName() === 'insert') {
50+
$command = $event->getCommand();
51+
$hasSession = isset($command->lsid);
52+
$writeConcern = isset($command->writeConcern) ? $command->writeConcern: null;
53+
54+
printf("insert command write concern: %s\n", json_encode($writeConcern));
55+
printf("insert command has session: %s\n", $hasSession ? 'yes' : 'no');
56+
}
57+
}
58+
59+
public function commandSucceeded(MongoDB\Driver\Monitoring\CommandSucceededEvent $event)
60+
{
61+
}
62+
63+
public function commandFailed(MongoDB\Driver\Monitoring\CommandFailedEvent $event)
64+
{
65+
}
66+
}
67+
68+
(new Test)->run();
69+
70+
?>
71+
===DONE===
72+
<?php exit(0); ?>
73+
--EXPECT--
74+
Testing executeBulkWrite
75+
insert command write concern: {"w":0}
76+
insert command has session: no
77+
78+
Testing executeWriteCommand
79+
insert command write concern: {"w":0}
80+
insert command has session: no
81+
82+
Testing executeReadWriteCommand
83+
insert command write concern: {"w":0}
84+
insert command has session: no
85+
===DONE===

‎tests/manager/manager-executeBulkWrite_error-010.phpt

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
--TEST--
2+
MongoDB\Driver\Manager::executeBulkWrite() cannot combine session with unacknowledged write concern
3+
--SKIPIF--
4+
<?php require __DIR__ . "/../utils/basic-skipif.inc"; ?>
5+
<?php NEEDS_CRYPTO(); ?>
6+
<?php NEEDS('STANDALONE'); ?>
7+
<?php NEEDS_ATLEAST_MONGODB_VERSION(STANDALONE, "3.6"); ?>
8+
--FILE--
9+
<?php
10+
require_once __DIR__ . "/../utils/basic.inc";
11+
12+
echo throws(function() {
13+
$manager = new MongoDB\Driver\Manager(STANDALONE);
14+
15+
$bulk = new MongoDB\Driver\BulkWrite;
16+
$bulk->insert(['x' => 1]);
17+
18+
$manager->executeBulkWrite(NS, $bulk, [
19+
'session' => $manager->startSession(),
20+
'writeConcern' => new MongoDB\Driver\WriteConcern(0),
21+
]);
22+
}, 'MongoDB\Driver\Exception\InvalidArgumentException'), "\n";
23+
24+
echo throws(function() {
25+
$manager = new MongoDB\Driver\Manager(STANDALONE, ['w' => 0]);
26+
27+
$bulk = new MongoDB\Driver\BulkWrite;
28+
$bulk->insert(['x' => 1]);
29+
30+
$manager->executeBulkWrite(NS, $bulk, [
31+
'session' => $manager->startSession(),
32+
]);
33+
}, 'MongoDB\Driver\Exception\InvalidArgumentException'), "\n";
34+
35+
?>
36+
===DONE===
37+
<?php exit(0); ?>
38+
--EXPECT--
39+
OK: Got MongoDB\Driver\Exception\InvalidArgumentException
40+
Cannot combine "session" option with an unacknowledged write concern
41+
OK: Got MongoDB\Driver\Exception\InvalidArgumentException
42+
Cannot combine "session" option with an unacknowledged write concern
43+
===DONE===

‎tests/manager/manager-executeCommand_error-003.phpt

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
--TEST--
2+
MongoDB\Driver\Manager::executeCommand() cannot combine session with unacknowledged write concern
3+
--SKIPIF--
4+
<?php require __DIR__ . "/../utils/basic-skipif.inc"; ?>
5+
<?php NEEDS_CRYPTO(); ?>
6+
<?php NEEDS('STANDALONE'); ?>
7+
<?php NEEDS_ATLEAST_MONGODB_VERSION(STANDALONE, "3.6"); ?>
8+
--FILE--
9+
<?php
10+
require_once __DIR__ . "/../utils/basic.inc";
11+
12+
echo throws(function() {
13+
$manager = new MongoDB\Driver\Manager(STANDALONE);
14+
15+
$command = new MongoDB\Driver\Command([
16+
'insert' => COLLECTION_NAME,
17+
'documents' => [['x' => 1]],
18+
]);
19+
20+
$manager->executeCommand(DATABASE_NAME, $command, [
21+
'session' => $manager->startSession(),
22+
'writeConcern' => new MongoDB\Driver\WriteConcern(0),
23+
]);
24+
}, 'MongoDB\Driver\Exception\InvalidArgumentException'), "\n";
25+
26+
?>
27+
===DONE===
28+
<?php exit(0); ?>
29+
--EXPECT--
30+
OK: Got MongoDB\Driver\Exception\InvalidArgumentException
31+
Cannot combine "session" option with an unacknowledged write concern
32+
===DONE===

‎tests/manager/manager-executeReadWriteCommand_error-002.phpt

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
--TEST--
2+
MongoDB\Driver\Manager::executeReadWriteCommand() cannot combine session with unacknowledged write concern
3+
--SKIPIF--
4+
<?php require __DIR__ . "/../utils/basic-skipif.inc"; ?>
5+
<?php NEEDS_CRYPTO(); ?>
6+
<?php NEEDS('STANDALONE'); ?>
7+
<?php NEEDS_ATLEAST_MONGODB_VERSION(STANDALONE, "3.6"); ?>
8+
--FILE--
9+
<?php
10+
require_once __DIR__ . "/../utils/basic.inc";
11+
12+
/* insert should not normally be used with executeReadWriteCommand(), but we are
13+
* only testing executeReadWriteCommand()'s option validation. */
14+
echo throws(function() {
15+
$manager = new MongoDB\Driver\Manager(STANDALONE);
16+
17+
$command = new MongoDB\Driver\Command([
18+
'insert' => COLLECTION_NAME,
19+
'documents' => [['x' => 1]],
20+
]);
21+
22+
$manager->executeReadWriteCommand(DATABASE_NAME, $command, [
23+
'session' => $manager->startSession(),
24+
'writeConcern' => new MongoDB\Driver\WriteConcern(0),
25+
]);
26+
}, 'MongoDB\Driver\Exception\InvalidArgumentException'), "\n";
27+
28+
echo throws(function() {
29+
$manager = new MongoDB\Driver\Manager(STANDALONE, ['w' => 0]);
30+
31+
$command = new MongoDB\Driver\Command([
32+
'insert' => COLLECTION_NAME,
33+
'documents' => [['x' => 1]],
34+
]);
35+
36+
$manager->executeReadWriteCommand(DATABASE_NAME, $command, [
37+
'session' => $manager->startSession(),
38+
]);
39+
}, 'MongoDB\Driver\Exception\InvalidArgumentException'), "\n";
40+
41+
?>
42+
===DONE===
43+
<?php exit(0); ?>
44+
--EXPECT--
45+
OK: Got MongoDB\Driver\Exception\InvalidArgumentException
46+
Cannot combine "session" option with an unacknowledged write concern
47+
OK: Got MongoDB\Driver\Exception\InvalidArgumentException
48+
Cannot combine "session" option with an unacknowledged write concern
49+
===DONE===

‎tests/manager/manager-executeWriteCommand_error-002.phpt

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
--TEST--
2+
MongoDB\Driver\Manager::executeWriteCommand() cannot combine session with unacknowledged write concern
3+
--SKIPIF--
4+
<?php require __DIR__ . "/../utils/basic-skipif.inc"; ?>
5+
<?php NEEDS_CRYPTO(); ?>
6+
<?php NEEDS('STANDALONE'); ?>
7+
<?php NEEDS_ATLEAST_MONGODB_VERSION(STANDALONE, "3.6"); ?>
8+
--FILE--
9+
<?php
10+
require_once __DIR__ . "/../utils/basic.inc";
11+
12+
echo throws(function() {
13+
$manager = new MongoDB\Driver\Manager(STANDALONE);
14+
15+
$command = new MongoDB\Driver\Command([
16+
'insert' => COLLECTION_NAME,
17+
'documents' => [['x' => 1]],
18+
]);
19+
20+
$manager->executeWriteCommand(DATABASE_NAME, $command, [
21+
'session' => $manager->startSession(),
22+
'writeConcern' => new MongoDB\Driver\WriteConcern(0),
23+
]);
24+
}, 'MongoDB\Driver\Exception\InvalidArgumentException'), "\n";
25+
26+
echo throws(function() {
27+
$manager = new MongoDB\Driver\Manager(STANDALONE, ['w' => 0]);
28+
29+
$command = new MongoDB\Driver\Command([
30+
'insert' => COLLECTION_NAME,
31+
'documents' => [['x' => 1]],
32+
]);
33+
34+
$manager->executeWriteCommand(DATABASE_NAME, $command, [
35+
'session' => $manager->startSession(),
36+
]);
37+
}, 'MongoDB\Driver\Exception\InvalidArgumentException'), "\n";
38+
39+
?>
40+
===DONE===
41+
<?php exit(0); ?>
42+
--EXPECT--
43+
OK: Got MongoDB\Driver\Exception\InvalidArgumentException
44+
Cannot combine "session" option with an unacknowledged write concern
45+
OK: Got MongoDB\Driver\Exception\InvalidArgumentException
46+
Cannot combine "session" option with an unacknowledged write concern
47+
===DONE===

0 commit comments

Comments
 (0)