Skip to content

PHPC-1163: Prohibit session with unacknowledged write concern #814

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 24, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 41 additions & 15 deletions php_phongo.c
Original file line number Diff line number Diff line change
Expand Up @@ -673,6 +673,17 @@ bool phongo_execute_bulk_write(mongoc_client_t *client, const char *namespace, p
return false;
}

/* If a write concern was not specified, libmongoc will use the client's
* write concern; however, we should still fetch it for the write result.
* Additionally, we need to check if an unacknowledged write concern would
* conflict with an explicit session. */
write_concern = zwriteConcern ? Z_WRITECONCERN_OBJ_P(zwriteConcern)->write_concern : mongoc_client_get_write_concern(client);

if (zsession && !mongoc_write_concern_is_acknowledged(write_concern)) {
phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT TSRMLS_CC, "Cannot combine \"session\" option with an unacknowledged write concern");
return false;
}

mongoc_bulk_operation_set_database(bulk, bulk_write->database);
mongoc_bulk_operation_set_collection(bulk, bulk_write->collection);
mongoc_bulk_operation_set_client(bulk, client);
Expand All @@ -682,13 +693,8 @@ bool phongo_execute_bulk_write(mongoc_client_t *client, const char *namespace, p
mongoc_bulk_operation_set_client_session(bulk, Z_SESSION_OBJ_P(zsession)->client_session);
}

/* If a write concern was not specified, libmongoc will use the client's
* write concern; however, we should still fetch it for the write result. */
write_concern = phongo_write_concern_from_zval(zwriteConcern TSRMLS_CC);
if (write_concern) {
mongoc_bulk_operation_set_write_concern(bulk, write_concern);
} else {
write_concern = mongoc_client_get_write_concern(client);
if (zwriteConcern) {
mongoc_bulk_operation_set_write_concern(bulk, Z_WRITECONCERN_OBJ_P(zwriteConcern)->write_concern);
}

success = mongoc_bulk_operation_execute(bulk, &reply, &error);
Expand Down Expand Up @@ -862,6 +868,7 @@ bool phongo_execute_command(mongoc_client_t *client, php_phongo_command_type_t t
bool result = false;
bool free_reply = false;
bool free_zsession = false;
bool is_unacknowledged_write_concern = false;

command = Z_COMMAND_OBJ_P(zcommand);

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

/* If an explicit session was not provided, attempt to create an implicit
* client session (ignoring any errors). */
if (!zsession) {
if (type & PHONGO_OPTION_WRITE_CONCERN) {
zval *zwriteConcern = NULL;

if (!phongo_parse_write_concern(options, &opts, &zwriteConcern TSRMLS_CC)) {
/* Exception should already have been thrown */
goto cleanup;
}

/* Determine if the explicit or inherited write concern is
* unacknowledged so that we can ensure it does not conflict with an
* explicit or implicit session. */
if (zwriteConcern) {
is_unacknowledged_write_concern = !mongoc_write_concern_is_acknowledged(Z_WRITECONCERN_OBJ_P(zwriteConcern)->write_concern);
} else if (type != PHONGO_COMMAND_RAW) {
is_unacknowledged_write_concern = !mongoc_write_concern_is_acknowledged(mongoc_client_get_write_concern(client));
}
}

if (zsession && is_unacknowledged_write_concern) {
phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT TSRMLS_CC, "Cannot combine \"session\" option with an unacknowledged write concern");
goto cleanup;
}

/* If an explicit session was not provided and the effective write concern
* is not unacknowledged, attempt to create an implicit client session
* (ignoring any errors). */
if (!zsession && !is_unacknowledged_write_concern) {
zsession = phongo_create_implicit_session(client TSRMLS_CC);

if (zsession) {
Expand All @@ -895,11 +926,6 @@ bool phongo_execute_command(mongoc_client_t *client, php_phongo_command_type_t t
}
}

if ((type & PHONGO_OPTION_WRITE_CONCERN) && !phongo_parse_write_concern(options, &opts, NULL TSRMLS_CC)) {
/* Exception should already have been thrown */
goto cleanup;
}

if (!BSON_APPEND_INT32(&opts, "serverId", server_id)) {
phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT TSRMLS_CC, "Error appending \"serverId\" option");
goto cleanup;
Expand Down
85 changes: 85 additions & 0 deletions tests/manager/bug1163-001.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
--TEST--
PHPC-1163: Unacknowledged write concern should omit implicit session
--XFAIL--
Depends on CDRIVER-2615
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests pass locally after applying the patch from CDRIVER-2615, but feel free to verify on your own. In any event, that fix won't show up until libmongoc 1.10.

--SKIPIF--
<?php if (PHP_INT_SIZE !== 8) { die("skip Can't represent 64-bit ints on a 32-bit platform"); } ?>
<?php require __DIR__ . "/../utils/basic-skipif.inc"; ?>
<?php NEEDS('STANDALONE'); ?>
<?php CLEANUP(STANDALONE); ?>
--FILE--
<?php
require_once __DIR__ . "/../utils/basic.inc";

class Test implements MongoDB\Driver\Monitoring\CommandSubscriber
{
public function run()
{
$manager = new MongoDB\Driver\Manager(STANDALONE, ['w' => 0]);

MongoDB\Driver\Monitoring\addSubscriber($this);

$bulk = new MongoDB\Driver\BulkWrite;
$bulk->insert(['x' => 1]);

echo "Testing executeBulkWrite\n";
$manager->executeBulkWrite(NS, $bulk);

$command = new MongoDB\Driver\Command([
'insert' => COLLECTION_NAME,
'documents' => [['x' => 1]],
]);

/* Note: executeCommand() and executeReadCommand() are not tested
* because they do not inherit the client-level write concern. */

echo "\nTesting executeWriteCommand\n";
$manager->executeWriteCommand(DATABASE_NAME, $command);

/* We can safely re-use the insert command with executeReadWriteCommand
* because there is no readConcern to inherit. */
echo "\nTesting executeReadWriteCommand\n";
$manager->executeReadWriteCommand(DATABASE_NAME, $command);

MongoDB\Driver\Monitoring\removeSubscriber($this);
}

public function commandStarted(MongoDB\Driver\Monitoring\CommandStartedEvent $event)
{
if ($event->getCommandName() === 'insert') {
$command = $event->getCommand();
$hasSession = isset($command->lsid);
$writeConcern = isset($command->writeConcern) ? $command->writeConcern: null;

printf("insert command write concern: %s\n", json_encode($writeConcern));
printf("insert command has session: %s\n", $hasSession ? 'yes' : 'no');
}
}

public function commandSucceeded(MongoDB\Driver\Monitoring\CommandSucceededEvent $event)
{
}

public function commandFailed(MongoDB\Driver\Monitoring\CommandFailedEvent $event)
{
}
}

(new Test)->run();

?>
===DONE===
<?php exit(0); ?>
--EXPECT--
Testing executeBulkWrite
insert command write concern: {"w":0}
insert command has session: no

Testing executeWriteCommand
insert command write concern: {"w":0}
insert command has session: no

Testing executeReadWriteCommand
insert command write concern: {"w":0}
insert command has session: no
===DONE===
43 changes: 43 additions & 0 deletions tests/manager/manager-executeBulkWrite_error-010.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
--TEST--
MongoDB\Driver\Manager::executeBulkWrite() cannot combine session with unacknowledged write concern
--SKIPIF--
<?php require __DIR__ . "/../utils/basic-skipif.inc"; ?>
<?php NEEDS_CRYPTO(); ?>
<?php NEEDS('STANDALONE'); ?>
<?php NEEDS_ATLEAST_MONGODB_VERSION(STANDALONE, "3.6"); ?>
--FILE--
<?php
require_once __DIR__ . "/../utils/basic.inc";

echo throws(function() {
$manager = new MongoDB\Driver\Manager(STANDALONE);

$bulk = new MongoDB\Driver\BulkWrite;
$bulk->insert(['x' => 1]);

$manager->executeBulkWrite(NS, $bulk, [
'session' => $manager->startSession(),
'writeConcern' => new MongoDB\Driver\WriteConcern(0),
]);
}, 'MongoDB\Driver\Exception\InvalidArgumentException'), "\n";

echo throws(function() {
$manager = new MongoDB\Driver\Manager(STANDALONE, ['w' => 0]);

$bulk = new MongoDB\Driver\BulkWrite;
$bulk->insert(['x' => 1]);

$manager->executeBulkWrite(NS, $bulk, [
'session' => $manager->startSession(),
]);
}, 'MongoDB\Driver\Exception\InvalidArgumentException'), "\n";

?>
===DONE===
<?php exit(0); ?>
--EXPECT--
OK: Got MongoDB\Driver\Exception\InvalidArgumentException
Cannot combine "session" option with an unacknowledged write concern
OK: Got MongoDB\Driver\Exception\InvalidArgumentException
Cannot combine "session" option with an unacknowledged write concern
===DONE===
32 changes: 32 additions & 0 deletions tests/manager/manager-executeCommand_error-003.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
--TEST--
MongoDB\Driver\Manager::executeCommand() cannot combine session with unacknowledged write concern
--SKIPIF--
<?php require __DIR__ . "/../utils/basic-skipif.inc"; ?>
<?php NEEDS_CRYPTO(); ?>
<?php NEEDS('STANDALONE'); ?>
<?php NEEDS_ATLEAST_MONGODB_VERSION(STANDALONE, "3.6"); ?>
--FILE--
<?php
require_once __DIR__ . "/../utils/basic.inc";

echo throws(function() {
$manager = new MongoDB\Driver\Manager(STANDALONE);

$command = new MongoDB\Driver\Command([
'insert' => COLLECTION_NAME,
'documents' => [['x' => 1]],
]);

$manager->executeCommand(DATABASE_NAME, $command, [
'session' => $manager->startSession(),
'writeConcern' => new MongoDB\Driver\WriteConcern(0),
]);
}, 'MongoDB\Driver\Exception\InvalidArgumentException'), "\n";

?>
===DONE===
<?php exit(0); ?>
--EXPECT--
OK: Got MongoDB\Driver\Exception\InvalidArgumentException
Cannot combine "session" option with an unacknowledged write concern
===DONE===
49 changes: 49 additions & 0 deletions tests/manager/manager-executeReadWriteCommand_error-002.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
--TEST--
MongoDB\Driver\Manager::executeReadWriteCommand() cannot combine session with unacknowledged write concern
--SKIPIF--
<?php require __DIR__ . "/../utils/basic-skipif.inc"; ?>
<?php NEEDS_CRYPTO(); ?>
<?php NEEDS('STANDALONE'); ?>
<?php NEEDS_ATLEAST_MONGODB_VERSION(STANDALONE, "3.6"); ?>
--FILE--
<?php
require_once __DIR__ . "/../utils/basic.inc";

/* insert should not normally be used with executeReadWriteCommand(), but we are
* only testing executeReadWriteCommand()'s option validation. */
echo throws(function() {
$manager = new MongoDB\Driver\Manager(STANDALONE);

$command = new MongoDB\Driver\Command([
'insert' => COLLECTION_NAME,
'documents' => [['x' => 1]],
]);

$manager->executeReadWriteCommand(DATABASE_NAME, $command, [
'session' => $manager->startSession(),
'writeConcern' => new MongoDB\Driver\WriteConcern(0),
]);
}, 'MongoDB\Driver\Exception\InvalidArgumentException'), "\n";

echo throws(function() {
$manager = new MongoDB\Driver\Manager(STANDALONE, ['w' => 0]);

$command = new MongoDB\Driver\Command([
'insert' => COLLECTION_NAME,
'documents' => [['x' => 1]],
]);

$manager->executeReadWriteCommand(DATABASE_NAME, $command, [
'session' => $manager->startSession(),
]);
}, 'MongoDB\Driver\Exception\InvalidArgumentException'), "\n";

?>
===DONE===
<?php exit(0); ?>
--EXPECT--
OK: Got MongoDB\Driver\Exception\InvalidArgumentException
Cannot combine "session" option with an unacknowledged write concern
OK: Got MongoDB\Driver\Exception\InvalidArgumentException
Cannot combine "session" option with an unacknowledged write concern
===DONE===
47 changes: 47 additions & 0 deletions tests/manager/manager-executeWriteCommand_error-002.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
--TEST--
MongoDB\Driver\Manager::executeWriteCommand() cannot combine session with unacknowledged write concern
--SKIPIF--
<?php require __DIR__ . "/../utils/basic-skipif.inc"; ?>
<?php NEEDS_CRYPTO(); ?>
<?php NEEDS('STANDALONE'); ?>
<?php NEEDS_ATLEAST_MONGODB_VERSION(STANDALONE, "3.6"); ?>
--FILE--
<?php
require_once __DIR__ . "/../utils/basic.inc";

echo throws(function() {
$manager = new MongoDB\Driver\Manager(STANDALONE);

$command = new MongoDB\Driver\Command([
'insert' => COLLECTION_NAME,
'documents' => [['x' => 1]],
]);

$manager->executeWriteCommand(DATABASE_NAME, $command, [
'session' => $manager->startSession(),
'writeConcern' => new MongoDB\Driver\WriteConcern(0),
]);
}, 'MongoDB\Driver\Exception\InvalidArgumentException'), "\n";

echo throws(function() {
$manager = new MongoDB\Driver\Manager(STANDALONE, ['w' => 0]);

$command = new MongoDB\Driver\Command([
'insert' => COLLECTION_NAME,
'documents' => [['x' => 1]],
]);

$manager->executeWriteCommand(DATABASE_NAME, $command, [
'session' => $manager->startSession(),
]);
}, 'MongoDB\Driver\Exception\InvalidArgumentException'), "\n";

?>
===DONE===
<?php exit(0); ?>
--EXPECT--
OK: Got MongoDB\Driver\Exception\InvalidArgumentException
Cannot combine "session" option with an unacknowledged write concern
OK: Got MongoDB\Driver\Exception\InvalidArgumentException
Cannot combine "session" option with an unacknowledged write concern
===DONE===