From 0bfde7d162264cd5f3813b7eddc721acb4bb5a03 Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Mon, 12 Sep 2016 16:42:10 -0400 Subject: [PATCH 1/2] Use correct type and format for ReadPreference ctor mode --- src/MongoDB/ReadPreference.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/MongoDB/ReadPreference.c b/src/MongoDB/ReadPreference.c index 88b171a05..b7d7aa6e0 100644 --- a/src/MongoDB/ReadPreference.c +++ b/src/MongoDB/ReadPreference.c @@ -52,7 +52,7 @@ PHP_METHOD(ReadPreference, __construct) { php_phongo_readpreference_t *intern; zend_error_handling error_handling; - long mode; + phongo_long mode; zval *tagSets = NULL; SUPPRESS_UNUSED_WARNING(return_value_ptr) SUPPRESS_UNUSED_WARNING(return_value) SUPPRESS_UNUSED_WARNING(return_value_used) @@ -88,7 +88,7 @@ PHP_METHOD(ReadPreference, __construct) } break; default: - phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT TSRMLS_CC, "Invalid mode: %ld", mode); + phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT TSRMLS_CC, "Invalid mode: %" PHONGO_LONG_FORMAT, mode); return; } } From 56d8b77b159e7381abea8104ce59f89067b87ad1 Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Mon, 12 Sep 2016 16:52:16 -0400 Subject: [PATCH 2/2] PHPC-424: Validate that RP tag set is an array of documents This adds common validation for read preference tag sets when specified through either the Manager constructor's URI options array or ReadPreference constructor. An additional test case for a malformed tag set has been added to the Manager::__construct() error test for read preference options. Additionally, the ReadPreference::__construct() error test has been split up to test for mode and tagSet errors separately. Note: we cannot test for the exceptions for bson_init_static() and mongoc_read_prefs_is_valid(), since those points will never be hit in normal operation. --- php_phongo.c | 39 ++++++++++++++++++- php_phongo.h | 2 + src/MongoDB/ReadPreference.c | 33 ++++++++++++---- tests/manager/manager-ctor_error-003.phpt | 10 ++++- .../readpreference-ctor_error-001.phpt | 10 +---- .../readpreference-ctor_error-002.phpt | 31 +++++++++++++++ 6 files changed, 105 insertions(+), 20 deletions(-) create mode 100644 tests/readPreference/readpreference-ctor_error-002.phpt diff --git a/php_phongo.c b/php_phongo.c index 4ca06f8e6..39522c050 100644 --- a/php_phongo.c +++ b/php_phongo.c @@ -936,6 +936,29 @@ void php_phongo_read_concern_to_zval(zval *retval, const mongoc_read_concern_t * } } /* }}} */ +/* Checks if tags is valid to set on a mongoc_read_prefs_t. It may be null or an + * array of one or more documents. */ +bool php_phongo_read_preference_tags_are_valid(const bson_t *tags) /* {{{ */ +{ + bson_iter_t iter; + + if (bson_empty0(tags)) { + return true; + } + + if (!bson_iter_init(&iter, tags)) { + return false; + } + + while (bson_iter_next(&iter)) { + if (!BSON_ITER_HOLDS_DOCUMENT(&iter) && !BSON_ITER_HOLDS_ARRAY(&iter)) { + return false; + } + } + + return true; +} /* }}} */ + void php_phongo_read_preference_to_zval(zval *retval, const mongoc_read_prefs_t *read_prefs) /* {{{ */ { const bson_t *tags = mongoc_read_prefs_get_tags(read_prefs); @@ -1143,9 +1166,21 @@ static bool php_phongo_apply_rp_options_to_uri(mongoc_uri_t *uri, bson_t *option bson_iter_array(&iter, &len, &data); - if (bson_init_static(&tags, data, len)) { - mongoc_read_prefs_set_tags(new_rp, &tags); + if (!bson_init_static(&tags, data, len)) { + phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT TSRMLS_CC, "Could not initialize BSON structure for read preference tags"); + mongoc_read_prefs_destroy(new_rp); + + return false; + } + + if (!php_phongo_read_preference_tags_are_valid(&tags)) { + phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT TSRMLS_CC, "Read preference tags must be an array of zero or more documents"); + mongoc_read_prefs_destroy(new_rp); + + return false; } + + mongoc_read_prefs_set_tags(new_rp, &tags); } if (mongoc_read_prefs_get_mode(new_rp) == MONGOC_READ_PRIMARY && diff --git a/php_phongo.h b/php_phongo.h index 341d738aa..55cf15efd 100644 --- a/php_phongo.h +++ b/php_phongo.h @@ -129,6 +129,8 @@ const mongoc_write_concern_t* phongo_write_concern_from_zval (zval *zwrite_conc php_phongo_server_description_type_t php_phongo_server_description_type(mongoc_server_description_t *sd); +bool php_phongo_read_preference_tags_are_valid(const bson_t *tags); + void php_phongo_server_to_zval(zval *retval, mongoc_server_description_t *sd); void php_phongo_read_concern_to_zval(zval *retval, const mongoc_read_concern_t *read_concern); void php_phongo_read_preference_to_zval(zval *retval, const mongoc_read_prefs_t *read_prefs); diff --git a/src/MongoDB/ReadPreference.c b/src/MongoDB/ReadPreference.c index b7d7aa6e0..4e2a01111 100644 --- a/src/MongoDB/ReadPreference.c +++ b/src/MongoDB/ReadPreference.c @@ -74,22 +74,39 @@ PHP_METHOD(ReadPreference, __construct) case MONGOC_READ_SECONDARY_PREFERRED: case MONGOC_READ_NEAREST: intern->read_preference = mongoc_read_prefs_new(mode); + break; + default: + phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT TSRMLS_CC, "Invalid mode: %" PHONGO_LONG_FORMAT, mode); + return; + } + switch(ZEND_NUM_ARGS()) { + case 2: if (tagSets) { bson_t *tags = bson_new(); phongo_zval_to_bson(tagSets, PHONGO_BSON_NONE, (bson_t *)tags, NULL TSRMLS_CC); - mongoc_read_prefs_set_tags(intern->read_preference, tags); - bson_destroy(tags); - if (!mongoc_read_prefs_is_valid(intern->read_preference)) { - phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT TSRMLS_CC, "Invalid tagSets"); + + if (!php_phongo_read_preference_tags_are_valid(tags)) { + phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT TSRMLS_CC, "tagSets must be an array of zero or more documents"); + bson_destroy(tags); + return; + } + + if (!bson_empty(tags) && mode == MONGOC_READ_PRIMARY) { + phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT TSRMLS_CC, "tagSets may not be used with primary mode"); + bson_destroy(tags); return; } + + mongoc_read_prefs_set_tags(intern->read_preference, tags); + bson_destroy(tags); } - break; - default: - phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT TSRMLS_CC, "Invalid mode: %" PHONGO_LONG_FORMAT, mode); - return; + } + + if (!mongoc_read_prefs_is_valid(intern->read_preference)) { + phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT TSRMLS_CC, "Read preference is not valid"); + return; } } /* }}} */ diff --git a/tests/manager/manager-ctor_error-003.phpt b/tests/manager/manager-ctor_error-003.phpt index 878354ba6..fc125d861 100644 --- a/tests/manager/manager-ctor_error-003.phpt +++ b/tests/manager/manager-ctor_error-003.phpt @@ -11,11 +11,15 @@ echo throws(function() { }, "MongoDB\Driver\Exception\InvalidArgumentException"), "\n"; echo throws(function() { - $manager = new MongoDB\Driver\Manager(STANDALONE, array('readPreference' => 'nothing')); + $manager = new MongoDB\Driver\Manager(STANDALONE, ['readPreference' => 'nothing']); }, "MongoDB\Driver\Exception\InvalidArgumentException"), "\n"; echo throws(function() { - $manager = new MongoDB\Driver\Manager(STANDALONE . '/?readPreference=primary', array('readPreferenceTags' => array(array()))); + $manager = new MongoDB\Driver\Manager(STANDALONE . '/?readPreference=primary', ['readPreferenceTags' => [[]]]); +}, "MongoDB\Driver\Exception\InvalidArgumentException"), "\n"; + +echo throws(function() { + $manager = new MongoDB\Driver\Manager(STANDALONE . '/?readPreference=primary', ['readPreferenceTags' => ['invalid']]); }, "MongoDB\Driver\Exception\InvalidArgumentException"), "\n"; ?> @@ -28,4 +32,6 @@ OK: Got MongoDB\Driver\Exception\InvalidArgumentException Unsupported readPreference value: 'nothing' OK: Got MongoDB\Driver\Exception\InvalidArgumentException Primary read preference mode conflicts with tags +OK: Got MongoDB\Driver\Exception\InvalidArgumentException +Read preference tags must be an array of zero or more documents ===DONE=== diff --git a/tests/readPreference/readpreference-ctor_error-001.phpt b/tests/readPreference/readpreference-ctor_error-001.phpt index 139954794..fa5234e7e 100644 --- a/tests/readPreference/readpreference-ctor_error-001.phpt +++ b/tests/readPreference/readpreference-ctor_error-001.phpt @@ -1,15 +1,11 @@ --TEST-- -MongoDB\Driver\ReadPreference construction (invalid arguments) +MongoDB\Driver\ReadPreference construction (invalid mode) --SKIPIF-- --FILE-- "one"))); -}, 'MongoDB\Driver\Exception\InvalidArgumentException'), "\n"; - echo throws(function() { new MongoDB\Driver\ReadPreference(42); }, 'MongoDB\Driver\Exception\InvalidArgumentException'), "\n"; @@ -17,9 +13,7 @@ echo throws(function() { ?> ===DONE=== ---EXPECTF-- -OK: Got MongoDB\Driver\Exception\InvalidArgumentException -Invalid tagSets +--EXPECT-- OK: Got MongoDB\Driver\Exception\InvalidArgumentException Invalid mode: 42 ===DONE=== diff --git a/tests/readPreference/readpreference-ctor_error-002.phpt b/tests/readPreference/readpreference-ctor_error-002.phpt new file mode 100644 index 000000000..b637d4680 --- /dev/null +++ b/tests/readPreference/readpreference-ctor_error-002.phpt @@ -0,0 +1,31 @@ +--TEST-- +MongoDB\Driver\ReadPreference construction (invalid tagSets) +--SKIPIF-- + +--FILE-- + 'one']]); +}, 'MongoDB\Driver\Exception\InvalidArgumentException'), "\n"; + +echo throws(function() { + new MongoDB\Driver\ReadPreference(MongoDB\Driver\ReadPreference::RP_PRIMARY, ['invalid']); +}, 'MongoDB\Driver\Exception\InvalidArgumentException'), "\n"; + +echo throws(function() { + new MongoDB\Driver\ReadPreference(MongoDB\Driver\ReadPreference::RP_SECONDARY, ['invalid']); +}, 'MongoDB\Driver\Exception\InvalidArgumentException'), "\n"; + +?> +===DONE=== + +--EXPECT-- +OK: Got MongoDB\Driver\Exception\InvalidArgumentException +tagSets may not be used with primary mode +OK: Got MongoDB\Driver\Exception\InvalidArgumentException +tagSets must be an array of zero or more documents +OK: Got MongoDB\Driver\Exception\InvalidArgumentException +tagSets must be an array of zero or more documents +===DONE===