Skip to content

PHPC-2441: Remove deprecated Manager constructor options #1719

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.

Sign up for GitHub

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 6 commits into from
Oct 16, 2024
Merged

PHPC-2441: Remove deprecated Manager constructor options #1719

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
767fd49
Use canonical TLS options in tests
jmikola Oct 11, 2024
5a2c483
Remove tests for deprecated TLS options
jmikola Oct 11, 2024
9bf487e
Remove deprecated Manager constructor options
jmikola Oct 9, 2024
ca4396b
Update UPGRADE-2.0.md
jmikola Oct 14, 2024
9e4f24e
clang-format
jmikola Oct 15, 2024
afe3bcb
Fix test failures
jmikola Oct 15, 2024
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
6 changes: 6 additions & 0 deletions UPGRADE-2.0.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,9 @@ UPGRADE FROM 1.x to 2.0
* The `MongoDB\Driver\Manager` constructor now throws if the URI options array
includes a non-boolean value for an option expecting a boolean. This behavior
is now consistent with validation for other option types.
* Removed the following driver options from `MongoDB\Driver\Manager`:
`allow_invalid_hostname` (use `tlsAllowInvalidHostnames` URI option instead),
`ca_file` (use ``tlsCAFile`), `context`,
`pem_file` (use `tlsCertificateKeyFile`),
`pem_pwd` (use `tlsCertificateKeyFilePassword`), and
`weak_cert_validation` (use `tlsAllowInvalidCertificates`).
58 changes: 3 additions & 55 deletions src/MongoDB/Manager.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,53 +54,6 @@
*/
zend_class_entry* php_phongo_manager_ce;

/* Checks if driverOptions contains a stream context resource in the "context"
* key and incorporates any of its SSL options into the base array that did not
* already exist (i.e. array union). The "context" key is then unset from the
* base array.
*
* This handles the merging of any legacy SSL context options and also makes
* driverOptions suitable for serialization by removing the resource zval. */
static bool php_phongo_manager_merge_context_options(zval* zdriverOptions)
{
php_stream_context* context;
zval * zcontext, *zcontextOptions;

if (!php_array_existsc(zdriverOptions, "context")) {
return true;
}

zcontext = php_array_fetchc_deref(zdriverOptions, "context");
context = php_stream_context_from_zval(zcontext, 1);

if (!context) {
phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT, "\"context\" driver option is not a valid Stream-Context resource");
return false;
}

zcontextOptions = php_array_fetchc_array(&context->options, "ssl");

if (!zcontextOptions) {
phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT, "Stream-Context resource does not contain \"ssl\" options array");
return false;
}

/* When running PHP in debug mode, php_error_docref duplicates the current
* scope, leading to a COW violation in zend_hash_merge and
* zend_symtable_str_del (called by php_array_unsetc). This macro allows
* that violation in debug mode and is a NOOP when in non-debug. */
HT_ALLOW_COW_VIOLATION(Z_ARRVAL_P(zdriverOptions));

php_error_docref(NULL, E_DEPRECATED, "The \"context\" driver option is deprecated.");

/* Perform array union (see: add_function() in zend_operators.c) */
zend_hash_merge(Z_ARRVAL_P(zdriverOptions), Z_ARRVAL_P(zcontextOptions), zval_add_ref, 0);

php_array_unsetc(zdriverOptions, "context");

return true;
}

/* Prepare authMechanismProperties for BSON encoding by converting a boolean
* value for the "CANONICALIZE_HOST_NAME" option to a string.
*
Expand Down Expand Up @@ -253,9 +206,9 @@ static PHP_METHOD(MongoDB_Driver_Manager, __construct)

intern = Z_MANAGER_OBJ_P(getThis());

/* Separate the options and driverOptions zvals, since we may end up
* modifying them in php_phongo_manager_prep_uri_options() and
* php_phongo_manager_merge_context_options() below, respectively. */
/* Separate the options zval, since it may be modified in
* php_phongo_manager_prep_uri_options(). Also separate driverOptions, since
* it may be modified in php_phongo_manager_prepare_manager_for_hash(). */
PHONGO_PARSE_PARAMETERS_START(0, 3)
Z_PARAM_OPTIONAL
Z_PARAM_STRING_OR_NULL(uri_string, uri_string_len)
Expand All @@ -267,11 +220,6 @@ static PHP_METHOD(MongoDB_Driver_Manager, __construct)
php_phongo_manager_prep_uri_options(options);
}

if (driverOptions && !php_phongo_manager_merge_context_options(driverOptions)) {
/* Exception should already have been thrown */
return;
}

phongo_manager_init(intern, uri_string ? uri_string : PHONGO_MANAGER_URI_DEFAULT, options, driverOptions);

if (EG(exception)) {
Expand Down
180 changes: 28 additions & 152 deletions src/phongo_client.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -544,6 +544,8 @@ static bool php_phongo_apply_wc_options_to_uri(mongoc_uri_t* uri, bson_t* option
while (bson_iter_next(&iter)) {
const char* key = bson_iter_key(&iter);

/* Note: although "safe" is deprecated and undocumented, we still handle
* it here for consistency with _mongoc_uri_build_write_concern() */
if (!ignore_safe && !strcasecmp(key, MONGOC_URI_SAFE)) {
if (!BSON_ITER_HOLDS_BOOL(&iter)) {
PHONGO_URI_INVALID_TYPE(iter, "boolean");
Expand Down Expand Up @@ -678,81 +680,64 @@ static void php_phongo_mongoc_ssl_opts_from_uri(mongoc_ssl_opt_t* ssl_opt, mongo
}
}

static inline char* php_phongo_fetch_ssl_opt_string(zval* zoptions, const char* key)
/* This function abstracts php_array_fetch_string() and always returns a string
* that must be freed by the caller. */
static inline char* php_phongo_fetch_string(zval* zarr, const char* key)
{
int plen;
zend_bool pfree;
char* pval;
char* value;

pval = php_array_fetch_string(zoptions, key, &plen, &pfree);
value = pfree ? pval : estrndup(pval, plen);

return value;
value = php_array_fetch_string(zarr, key, &plen, &pfree);

return pfree ? value : estrndup(value, plen);
}

static mongoc_ssl_opt_t* php_phongo_make_ssl_opt(mongoc_uri_t* uri, zval* zoptions)
static mongoc_ssl_opt_t* php_phongo_make_ssl_opt(mongoc_uri_t* uri, zval* driverOptions)
{
mongoc_ssl_opt_t* ssl_opt;
bool any_ssl_option_set = false;

if (!zoptions) {
if (!driverOptions) {
return NULL;
}

#if defined(MONGOC_ENABLE_SSL_SECURE_CHANNEL) || defined(MONGOC_ENABLE_SSL_SECURE_TRANSPORT)
if (php_array_existsc(zoptions, "ca_dir")) {
if (php_array_existsc(driverOptions, "ca_dir")) {
phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT, "\"ca_dir\" option is not supported by Secure Channel and Secure Transport");
return NULL;
}

if (php_array_existsc(zoptions, "capath")) {
phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT, "\"capath\" option is not supported by Secure Channel and Secure Transport");
return NULL;
}
#endif

#if defined(MONGOC_ENABLE_SSL_LIBRESSL) || defined(MONGOC_ENABLE_SSL_SECURE_TRANSPORT)
if (php_array_existsc(zoptions, "crl_file")) {
if (php_array_existsc(driverOptions, "crl_file")) {
phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT, "\"crl_file\" option is not supported by LibreSSL and Secure Transport");
return NULL;
}
#endif

/* Note: consider copying from mongoc_ssl_opt_get_default() if
* MONGOC_SSL_DEFAULT_TRUST_FILE and MONGOC_SSL_DEFAULT_TRUST_DIR are ever
* used, but libmongoc currently defines them as null. */
ssl_opt = ecalloc(1, sizeof(mongoc_ssl_opt_t));

/* If SSL options are set in the URL, we need to read them and set them on
* the options struct so we can merge potential options from passed in
* driverOptions (zoptions) */
/* Apply TLS options to the ssl_opt struct before driver options */
if (mongoc_uri_get_tls(uri)) {
php_phongo_mongoc_ssl_opts_from_uri(ssl_opt, uri, &any_ssl_option_set);
}

#define PHONGO_SSL_OPTION_SWAP_STRING(o, n) \
if ((o)) { \
efree((char*) (o)); \
} \
(o) = php_phongo_fetch_ssl_opt_string(zoptions, n);

/* Apply driver options that don't have a corresponding URI option. These
* are set directly on the SSL options struct. */
if (php_array_existsc(zoptions, "ca_dir")) {
PHONGO_SSL_OPTION_SWAP_STRING(ssl_opt->ca_dir, "ca_dir");
any_ssl_option_set = true;
} else if (php_array_existsc(zoptions, "capath")) {
PHONGO_SSL_OPTION_SWAP_STRING(ssl_opt->ca_dir, "capath");
if (php_array_existsc(driverOptions, "ca_dir")) {
ssl_opt->ca_dir = php_phongo_fetch_string(driverOptions, "ca_dir");
any_ssl_option_set = true;

php_error_docref(NULL, E_DEPRECATED, "The \"capath\" context driver option is deprecated. Please use the \"ca_dir\" driver option instead.");
}

if (php_array_existsc(zoptions, "crl_file")) {
PHONGO_SSL_OPTION_SWAP_STRING(ssl_opt->crl_file, "crl_file");
if (php_array_existsc(driverOptions, "crl_file")) {
ssl_opt->crl_file = php_phongo_fetch_string(driverOptions, "crl_file");
any_ssl_option_set = true;
}

#undef PHONGO_SSL_OPTION_SWAP_STRING

if (!any_ssl_option_set) {
efree(ssl_opt);
return NULL;
Expand Down Expand Up @@ -785,110 +770,6 @@ static void php_phongo_free_ssl_opt(mongoc_ssl_opt_t* ssl_opt)

efree(ssl_opt);
}

static inline bool php_phongo_apply_driver_option_to_uri(mongoc_uri_t* uri, zval* zoptions, const char* driverOptionKey, const char* optionKey)
{
bool ret;
char* value;

value = php_phongo_fetch_ssl_opt_string(zoptions, driverOptionKey);
ret = mongoc_uri_set_option_as_utf8(uri, optionKey, value);
efree(value);

return ret;
}

static bool php_phongo_apply_driver_options_to_uri(mongoc_uri_t* uri, zval* zoptions)
{
if (!zoptions) {
return true;
}

/* Map TLS driver options to the canonical tls options in the URI. */
if (php_array_existsc(zoptions, "allow_invalid_hostname")) {
if (!mongoc_uri_set_option_as_bool(uri, MONGOC_URI_TLSALLOWINVALIDHOSTNAMES, php_array_fetchc_bool(zoptions, "allow_invalid_hostname"))) {
phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT, "Failed to parse \"%s\" driver option", "allow_invalid_hostname");

return false;
}

php_error_docref(NULL, E_DEPRECATED, "The \"allow_invalid_hostname\" driver option is deprecated. Please use the \"tlsAllowInvalidHostnames\" URI option instead.");
}

if (php_array_existsc(zoptions, "weak_cert_validation")) {
if (!mongoc_uri_set_option_as_bool(uri, MONGOC_URI_TLSALLOWINVALIDCERTIFICATES, php_array_fetchc_bool(zoptions, "weak_cert_validation"))) {
phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT, "Failed to parse \"%s\" driver option", "weak_cert_validation");

return false;
}

php_error_docref(NULL, E_DEPRECATED, "The \"weak_cert_validation\" driver option is deprecated. Please use the \"tlsAllowInvalidCertificates\" URI option instead.");
} else if (php_array_existsc(zoptions, "allow_self_signed")) {
if (!mongoc_uri_set_option_as_bool(uri, MONGOC_URI_TLSALLOWINVALIDCERTIFICATES, php_array_fetchc_bool(zoptions, "allow_self_signed"))) {
phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT, "Failed to parse \"%s\" driver option", "allow_self_signed");

return false;
}

php_error_docref(NULL, E_DEPRECATED, "The \"allow_self_signed\" context driver option is deprecated. Please use the \"tlsAllowInvalidCertificates\" URI option instead.");
}

if (php_array_existsc(zoptions, "pem_file")) {
if (!php_phongo_apply_driver_option_to_uri(uri, zoptions, "pem_file", MONGOC_URI_TLSCERTIFICATEKEYFILE)) {
phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT, "Failed to parse \"%s\" driver option", "pem_file");

return false;
}

php_error_docref(NULL, E_DEPRECATED, "The \"pem_file\" driver option is deprecated. Please use the \"tlsCertificateKeyFile\" URI option instead.");
} else if (php_array_existsc(zoptions, "local_cert")) {
if (!php_phongo_apply_driver_option_to_uri(uri, zoptions, "local_cert", MONGOC_URI_TLSCERTIFICATEKEYFILE)) {
phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT, "Failed to parse \"%s\" driver option", "local_cert");

return false;
}

php_error_docref(NULL, E_DEPRECATED, "The \"local_cert\" context driver option is deprecated. Please use the \"tlsCertificateKeyFile\" URI option instead.");
}

if (php_array_existsc(zoptions, "pem_pwd")) {
if (!php_phongo_apply_driver_option_to_uri(uri, zoptions, "pem_pwd", MONGOC_URI_TLSCERTIFICATEKEYFILEPASSWORD)) {
phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT, "Failed to parse \"%s\" driver option", "pem_pwd");

return false;
}

php_error_docref(NULL, E_DEPRECATED, "The \"pem_pwd\" driver option is deprecated. Please use the \"tlsCertificateKeyFilePassword\" URI option instead.");
} else if (php_array_existsc(zoptions, "passphrase")) {
if (!php_phongo_apply_driver_option_to_uri(uri, zoptions, "passphrase", MONGOC_URI_TLSCERTIFICATEKEYFILEPASSWORD)) {
phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT, "Failed to parse \"%s\" driver option", "passphrase");

return false;
}

php_error_docref(NULL, E_DEPRECATED, "The \"passphrase\" context driver option is deprecated. Please use the \"tlsCertificateKeyFilePassword\" URI option instead.");
}

if (php_array_existsc(zoptions, "ca_file")) {
if (!php_phongo_apply_driver_option_to_uri(uri, zoptions, "ca_file", MONGOC_URI_TLSCAFILE)) {
phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT, "Failed to parse \"%s\" driver option", "ca_file");

return false;
}

php_error_docref(NULL, E_DEPRECATED, "The \"ca_file\" driver option is deprecated. Please use the \"tlsCAFile\" URI option instead.");
} else if (php_array_existsc(zoptions, "cafile")) {
if (!php_phongo_apply_driver_option_to_uri(uri, zoptions, "cafile", MONGOC_URI_TLSCAFILE)) {
phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT, "Failed to parse \"%s\" driver option", "cafile");

return false;
}

php_error_docref(NULL, E_DEPRECATED, "The \"cafile\" context driver option is deprecated. Please use the \"tlsCAFile\" URI option instead.");
}

return true;
}
#endif /* MONGOC_ENABLE_SSL */

static zval* php_phongo_manager_prepare_manager_for_hash(zval* driverOptions, bool* free)
Expand All @@ -907,21 +788,21 @@ static zval* php_phongo_manager_prepare_manager_for_hash(zval* driverOptions, bo
}

if (!php_array_existsc(driverOptions, "autoEncryption")) {
goto ref;
return driverOptions;
}

autoEncryptionOpts = php_array_fetchc(driverOptions, "autoEncryption");
if (Z_TYPE_P(autoEncryptionOpts) != IS_ARRAY) {
goto ref;
return driverOptions;
}

if (!php_array_existsc(autoEncryptionOpts, "keyVaultClient")) {
goto ref;
return driverOptions;
}

keyVaultClient = php_array_fetchc(autoEncryptionOpts, "keyVaultClient");
if (Z_TYPE_P(keyVaultClient) != IS_OBJECT || !instanceof_function(Z_OBJCE_P(keyVaultClient), php_phongo_manager_ce)) {
goto ref;
return driverOptions;
}

*free = true;
Expand All @@ -938,10 +819,6 @@ static zval* php_phongo_manager_prepare_manager_for_hash(zval* driverOptions, bo
ADD_ASSOC_ZVAL_EX(driverOptionsClone, "autoEncryption", autoEncryptionOptsClone);

return driverOptionsClone;

ref:
Z_ADDREF_P(driverOptions);
Copy link
Member

Choose a reason for hiding this comment

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

Noted this was moved to php_phongo_manager_make_client_hash, where it makes more sense.

return driverOptions;
}

/* Creates a hash for a client by concatenating the URI string with serialized
Expand Down Expand Up @@ -972,6 +849,10 @@ static char* php_phongo_manager_make_client_hash(const char* uri_string, zval* o
if (driverOptions) {
serializable_driver_options = php_phongo_manager_prepare_manager_for_hash(driverOptions, &free_driver_options);
ADD_ASSOC_ZVAL_EX(&args, "driverOptions", serializable_driver_options);
/* Add a reference to driverOptions unless a new copy was returned */
if (!free_driver_options) {
Z_ADDREF_P(serializable_driver_options);
}
} else {
ADD_ASSOC_NULL_EX(&args, "driverOptions");
}
Expand Down Expand Up @@ -1504,11 +1385,6 @@ void phongo_manager_init(php_phongo_manager_t* manager, const char* uri_string,
}

#ifdef MONGOC_ENABLE_SSL
if (!php_phongo_apply_driver_options_to_uri(uri, driverOptions)) {
/* Exception should already have been thrown */
goto cleanup;
}

ssl_opt = php_phongo_make_ssl_opt(uri, driverOptions);

/* An exception may be thrown during SSL option creation */
Expand Down
13 changes: 5 additions & 8 deletions tests/connect/bug0720.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,13 @@ PHPC-720: Do not persist SSL streams to avoid SSL reinitialization errors
<?php
require_once __DIR__ . "/../utils/basic.inc";

$driverOptions = [
$uriOptions = [
// libmongoc does not allow the hostname to be overridden as "server"
'allow_invalid_hostname' => true,
'ca_file' => SSL_DIR . '/ca.pem',
'tlsAllowInvalidHostnames' => true,
'tlsCAFile' => SSL_DIR . '/ca.pem',
];

$manager = create_test_manager(URI, [], $driverOptions);
$manager = create_test_manager(URI, $uriOptions);
$cursor = $manager->executeCommand(DATABASE_NAME, new MongoDB\Driver\Command(['ping' => 1]));
printf("ping: %d\n", $cursor->toArray()[0]->ok);

Expand All @@ -29,10 +29,7 @@ printf("ping: %d\n", $cursor->toArray()[0]->ok);
?>
===DONE===
<?php exit(0); ?>
--EXPECTF--
Deprecated: MongoDB\Driver\Manager::__construct(): The "allow_invalid_hostname" driver option is deprecated. Please use the "tlsAllowInvalidHostnames" URI option instead.%s

Deprecated: MongoDB\Driver\Manager::__construct(): The "ca_file" driver option is deprecated. Please use the "tlsCAFile" URI option instead.%s
--EXPECT--
ping: 1
ping: 1
===DONE===
Loading
Loading