Skip to content
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
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,13 @@

## Unreleased

### Breaking Changes

* An AZ-affinity read strategy (`READ_FROM_AZ_AFFINITY`, `READ_FROM_AZ_AFFINITY_REPLICAS_AND_PRIMARY`, or `READ_FROM_AZ_AFFINITY_ALL_NODES`) configured without a valid `client_az` now throws at client creation. Previously the core logged a warning and downgraded the strategy to `PreferReplica`, so reads silently went to arbitrary nodes. A `client_az` that is empty, whitespace-only, contains a NUL byte, or has leading/trailing whitespace is rejected ([#316](https://github.com/valkey-io/valkey-glide-php/issues/316))

### Changes

* Add `READ_FROM_AZ_AFFINITY_ALL_NODES` read strategy for standalone and cluster clients ([#316](https://github.com/valkey-io/valkey-glide-php/issues/316))
Comment thread
prateek-kumar-improving marked this conversation as resolved.
* Add mutual TLS (mTLS) support for standalone and cluster clients via `advanced_config['tls_config']` — byte-based (`client_cert`/`client_key`) or path-based with automatic certificate reload (`client_cert_path`/`client_key_path` + optional `cert_reload_interval_seconds`) ([#321](https://github.com/valkey-io/valkey-glide-php/pull/321))
* Add `NodeDiscoveryMode` configuration (`STANDARD`, `STATIC`, `DISCOVER_ALL`) for standalone client ([#302](https://github.com/valkey-io/valkey-glide-php/issues/302))
* Add `CONFIG` command for cluster client ([#284](https://github.com/valkey-io/valkey-glide-php/issues/284))
Expand Down
3 changes: 2 additions & 1 deletion common.h
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,8 @@ typedef enum {
VALKEY_GLIDE_READ_FROM_PRIMARY = 0,
VALKEY_GLIDE_READ_FROM_PREFER_REPLICA = 1,
VALKEY_GLIDE_READ_FROM_AZ_AFFINITY = 2,
VALKEY_GLIDE_READ_FROM_AZ_AFFINITY_REPLICAS_AND_PRIMARY = 3
VALKEY_GLIDE_READ_FROM_AZ_AFFINITY_REPLICAS_AND_PRIMARY = 3,
VALKEY_GLIDE_READ_FROM_AZ_AFFINITY_ALL_NODES = 4
} valkey_glide_read_from_t;

typedef enum {
Expand Down
5 changes: 3 additions & 2 deletions examples/basic/configuration.php
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@
// All configuration options for standalone client
$use_tls = false;
$credentials = null; // ['username' => 'user', 'password' => 'pass']
$read_from = 0; // 0=PRIMARY, 1=PREFER_REPLICA, 2=AZ_AFFINITY
$read_from = 0; // 0=PRIMARY, 1=PREFER_REPLICA, 2=AZ_AFFINITY,
// 3=AZ_AFFINITY_REPLICAS_AND_PRIMARY, 4=AZ_AFFINITY_ALL_NODES
$request_timeout = 2000; // 2 seconds in milliseconds
$reconnect_strategy = [
'num_of_retries' => 3,
Expand All @@ -66,7 +67,7 @@
$database_id = 0; // Database number (0 or higher for standalone)
$client_name = 'valkey-glide-example';
$inflight_requests_limit = 250;
$client_az = null; // Availability zone for AZ_AFFINITY reads
$client_az = null; // Availability zone; required for AZ_AFFINITY, AZ_AFFINITY_REPLICAS_AND_PRIMARY, and AZ_AFFINITY_ALL_NODES reads
$advanced_config = [
'connection_timeout' => 5000, // Connection timeout in milliseconds
'socket_timeout' => 3000 // Socket timeout in milliseconds
Expand Down
175 changes: 173 additions & 2 deletions tests/ConnectionRequestTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -204,16 +204,187 @@ public function testClusterCredentials()

public function testStandaloneReadFrom()
{
$request = ClientConstructorMock::simulate_standalone_constructor(read_from: ValkeyGlide::READ_FROM_AZ_AFFINITY);
$request = ClientConstructorMock::simulate_standalone_constructor(
read_from: ValkeyGlide::READ_FROM_AZ_AFFINITY,
client_az: 'us-east-1a'
);
$this->assertEquals(\Connection_request\ReadFrom::AZAffinity, $request->getReadFrom());
}

public function testClusterReadFrom()
{
$request = ClientConstructorMock::simulate_cluster_constructor(read_from: ValkeyGlide::READ_FROM_AZ_AFFINITY_REPLICAS_AND_PRIMARY);
$request = ClientConstructorMock::simulate_cluster_constructor(
read_from: ValkeyGlide::READ_FROM_AZ_AFFINITY_REPLICAS_AND_PRIMARY,
client_az: 'us-east-1a'
);
$this->assertEquals(\Connection_request\ReadFrom::AZAffinityReplicasAndPrimary, $request->getReadFrom());
}

public function testStandaloneReadFromAzAffinityAllNodes()
{
$request = ClientConstructorMock::simulate_standalone_constructor(
read_from: ValkeyGlide::READ_FROM_AZ_AFFINITY_ALL_NODES,
client_az: 'us-east-1a'
);
$this->assertEquals(\Connection_request\ReadFrom::AZAffinityAllNodes, $request->getReadFrom());
}

public function testClusterReadFromAzAffinityAllNodes()
{
$request = ClientConstructorMock::simulate_cluster_constructor(
read_from: ValkeyGlide::READ_FROM_AZ_AFFINITY_ALL_NODES,
client_az: 'us-east-1a'
);
$this->assertEquals(\Connection_request\ReadFrom::AZAffinityAllNodes, $request->getReadFrom());
}

public function testStandaloneAzAffinityAllNodesRequiresClientAz()
{
$this->assertThrowsMatch(
null,
function () {
ClientConstructorMock::simulate_standalone_constructor(
read_from: ValkeyGlide::READ_FROM_AZ_AFFINITY_ALL_NODES
);
},
'/client_az must be set when read_from is set to AZ_AFFINITY_ALL_NODES/'
);
}

public function testClusterAzAffinityAllNodesRequiresClientAz()
{
$this->assertThrowsMatch(
null,
function () {
ClientConstructorMock::simulate_cluster_constructor(
read_from: ValkeyGlide::READ_FROM_AZ_AFFINITY_ALL_NODES
);
},
'/client_az must be set when read_from is set to AZ_AFFINITY_ALL_NODES/'
);
}

/**
* Every AZ-affinity strategy must require a client AZ, and each must report its own name in the
* error. Mirrors the reference clients, which validate all three strategies rather than only the
* newly added one, so a regression in any of them is caught here.
*/
public function testAzAffinityStrategiesRequireClientAz()
{
$strategies = [
ValkeyGlide::READ_FROM_AZ_AFFINITY => 'AZ_AFFINITY',
ValkeyGlide::READ_FROM_AZ_AFFINITY_REPLICAS_AND_PRIMARY => 'AZ_AFFINITY_REPLICAS_AND_PRIMARY',
ValkeyGlide::READ_FROM_AZ_AFFINITY_ALL_NODES => 'AZ_AFFINITY_ALL_NODES',
];

foreach ($strategies as $read_from => $name) {
$pattern = '/client_az must be set when read_from is set to ' . preg_quote($name, '/') . '/';

$this->assertThrowsMatch(
null,
function () use ($read_from) {
ClientConstructorMock::simulate_standalone_constructor(read_from: $read_from);
},
$pattern
);

$this->assertThrowsMatch(
null,
function () use ($read_from) {
ClientConstructorMock::simulate_cluster_constructor(read_from: $read_from);
},
$pattern
);
}
}

/**
* Non-AZ strategies never require a client AZ. Confirms the validation is scoped to exactly the
* AZ-affinity strategies and does not leak into the default/replica strategies.
*/
public function testNonAzStrategiesDoNotRequireClientAz()
{
$strategies = [
ValkeyGlide::READ_FROM_PRIMARY => \Connection_request\ReadFrom::Primary,
ValkeyGlide::READ_FROM_PREFER_REPLICA => \Connection_request\ReadFrom::PreferReplica,
];

foreach ($strategies as $read_from => $expected) {
$request = ClientConstructorMock::simulate_standalone_constructor(read_from: $read_from);
$this->assertEquals($expected, $request->getReadFrom());
}
}

/**
* A client_az that is empty or whitespace-only is treated as absent. The core compares AZs with
* exact equality and never trims, so a blank value would otherwise engage the strategy, match no
* node, and silently fall back to routing across all nodes. Rejecting it surfaces the
* misconfiguration at client creation instead.
*/
public function testAzAffinityAllNodesRejectsBlankClientAz()
{
foreach (['', ' ', ' ', "\t", "\n", " \t\n "] as $blank) {
$this->assertThrowsMatch(
null,
function () use ($blank) {
ClientConstructorMock::simulate_standalone_constructor(
read_from: ValkeyGlide::READ_FROM_AZ_AFFINITY_ALL_NODES,
client_az: $blank
);
},
'/client_az must be set when read_from is set to AZ_AFFINITY_ALL_NODES/'
);
}
}

/**
* A client_az containing a NUL byte is rejected outright. The value reaches the core as a
* NUL-terminated C string, so an embedded NUL would be silently truncated on the wire (e.g.
* "us-east-1a\0x" would arrive as "us-east-1a"), changing AZ routing without the caller's
* knowledge. A NUL is never part of a legitimate availability-zone name.
*/
public function testClientAzRejectsNulBytes()
{
$nul_values = ["\0", "\0us-east-1a", " \0us-east-1a", "us-east-1a\0", "us-east-1a\0x"];

foreach ($nul_values as $value) {
$this->assertThrowsMatch(
null,
function () use ($value) {
ClientConstructorMock::simulate_standalone_constructor(
read_from: ValkeyGlide::READ_FROM_AZ_AFFINITY_ALL_NODES,
client_az: $value
);
},
'/client_az must not contain NUL bytes/'
);
}
}

/**
* A client_az with leading or trailing whitespace (e.g. "us-east-1a\n" from getenv or
* file_get_contents) is rejected. It has content but would never match a node under the core's
* exact-equality compare, silently falling back to reading across all nodes. Rejecting it
* surfaces the misconfiguration at client creation instead.
*/
public function testClientAzRejectsSurroundingWhitespace()
{
$padded = ["us-east-1a\n", " us-east-1a", "us-east-1a ", "\tus-east-1a", " us-east-1a "];

foreach ($padded as $value) {
$this->assertThrowsMatch(
null,
function () use ($value) {
ClientConstructorMock::simulate_standalone_constructor(
read_from: ValkeyGlide::READ_FROM_AZ_AFFINITY_ALL_NODES,
client_az: $value
);
},
'/client_az must not have leading or trailing whitespace/'
);
}
}

public function testStandaloneNodeDiscoveryModeDefault()
{
$request = ClientConstructorMock::simulate_standalone_constructor();
Expand Down
6 changes: 4 additions & 2 deletions tests/ValkeyGlideClusterFeaturesTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,8 @@ public function testConstructorWithReadFromAzAffinity()
addresses: [['host' => '127.0.0.1', 'port' => 7001]],
use_tls: false,
credentials: $this->getAuth(),
read_from: ValkeyGlide::READ_FROM_AZ_AFFINITY
read_from: ValkeyGlide::READ_FROM_AZ_AFFINITY,
client_az: 'us-east-1a'
);

$this->assertTrue($valkey_glide->ping(['type' => 'primarySlotKey', 'key' => 'test']));
Expand All @@ -321,7 +322,8 @@ public function testConstructorWithReadFromAzAffinityReplicasAndPrimary()
addresses: [['host' => '127.0.0.1', 'port' => 7001]],
use_tls: false,
credentials: $this->getAuth(),
read_from: ValkeyGlide::READ_FROM_AZ_AFFINITY_REPLICAS_AND_PRIMARY
read_from: ValkeyGlide::READ_FROM_AZ_AFFINITY_REPLICAS_AND_PRIMARY,
client_az: 'us-east-1a'
);

$this->assertTrue($valkey_glide->ping(['type' => 'primarySlotKey', 'key' => 'test']));
Expand Down
8 changes: 4 additions & 4 deletions tests/ValkeyGlideFeaturesTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -597,27 +597,27 @@ public function testConstructorWithAzAffinityReadStrategy()
// Test READ_FROM_AZ_AFFINITY
if (!$this->getTLS()) {
$valkey_glide = new ValkeyGlide();
$valkey_glide->connect(addresses: $addresses, use_tls: $this->getTLS(), read_from: ValkeyGlide::READ_FROM_AZ_AFFINITY);
$valkey_glide->connect(addresses: $addresses, use_tls: $this->getTLS(), read_from: ValkeyGlide::READ_FROM_AZ_AFFINITY, client_az: 'us-east-1a');
} else {
$advancedConfig = [
'tls_config' => ['use_insecure_tls' => true]
];
$valkey_glide = new ValkeyGlide();
$valkey_glide->connect(addresses: $addresses, use_tls: true, read_from: ValkeyGlide::READ_FROM_AZ_AFFINITY, advanced_config: $advancedConfig);
$valkey_glide->connect(addresses: $addresses, use_tls: true, read_from: ValkeyGlide::READ_FROM_AZ_AFFINITY, client_az: 'us-east-1a', advanced_config: $advancedConfig);
}
$this->assertTrue($valkey_glide->ping());
$valkey_glide->close();

// Test READ_FROM_AZ_AFFINITY_REPLICAS_AND_PRIMARY
if (!$this->getTLS()) {
$valkey_glide = new ValkeyGlide();
$valkey_glide->connect(addresses: $addresses, use_tls: $this->getTLS(), read_from: ValkeyGlide::READ_FROM_AZ_AFFINITY_REPLICAS_AND_PRIMARY);
$valkey_glide->connect(addresses: $addresses, use_tls: $this->getTLS(), read_from: ValkeyGlide::READ_FROM_AZ_AFFINITY_REPLICAS_AND_PRIMARY, client_az: 'us-east-1a');
} else {
$advancedConfig = [
'tls_config' => ['use_insecure_tls' => true]
];
$valkey_glide = new ValkeyGlide();
$valkey_glide->connect(addresses: $addresses, use_tls: true, read_from: ValkeyGlide::READ_FROM_AZ_AFFINITY_REPLICAS_AND_PRIMARY, advanced_config: $advancedConfig);
$valkey_glide->connect(addresses: $addresses, use_tls: true, read_from: ValkeyGlide::READ_FROM_AZ_AFFINITY_REPLICAS_AND_PRIMARY, client_az: 'us-east-1a', advanced_config: $advancedConfig);
}
$this->assertTrue($valkey_glide->ping());
$valkey_glide->close();
Expand Down
81 changes: 79 additions & 2 deletions valkey_glide.c
Original file line number Diff line number Diff line change
Expand Up @@ -217,8 +217,55 @@ int valkey_glide_build_client_config_base(valkey_glide_php_common_constructor_pa
since it is effectively one-request-at-a-time. */
config->inflight_requests_limit = -1;

/* Set client availability zone */
config->client_az = (params->client_az && params->client_az_len > 0) ? params->client_az : NULL;
/* Set client availability zone.
*
* The value is forwarded to the core as a NUL-terminated C string, so an embedded NUL would be
* silently truncated on the wire (e.g. "us-east-1a\0x" would reach the core as "us-east-1a"),
* changing AZ-based routing without the caller's knowledge. A NUL is never part of a legitimate
* availability-zone name, so reject any value containing one.
*
* The core compares availability zones with exact equality and never trims. Therefore:
* - An empty or whitespace-only value is treated as absent (NULL); the AZ-affinity
* requirement below then rejects it, surfacing the misconfiguration at client creation instead
* of letting the strategy engage, match no node, and silently spread reads across all nodes.
* - A value with leading or trailing whitespace (e.g. "us-east-1a\n" from
* getenv/file_get_contents) has content but would never match a node under the core's
* exact-equality compare, producing the same silent all-nodes fallback. It is rejected rather
* than forwarded. Only the borrowed PHP pointer is forwarded, never a copy, so client config
* ownership is unchanged. */
config->client_az = NULL;
if (params->client_az && params->client_az_len > 0) {
bool az_has_content = false;
for (size_t az_i = 0; az_i < params->client_az_len; az_i++) {
char az_ch = params->client_az[az_i];
if (az_ch == '\0') {
const char* az_error_message = "client_az must not contain NUL bytes.";
VALKEY_LOG_ERROR("valkey_glide_build_client_config_base", az_error_message);
zend_throw_exception(get_valkey_glide_exception_ce(), az_error_message, 0);
return FAILURE;
}
if (az_ch != ' ' && az_ch != '\t' && az_ch != '\n' && az_ch != '\r' && az_ch != '\f' &&
az_ch != '\v') {
az_has_content = true;
}
}
if (az_has_content) {
char first_ch = params->client_az[0];
char last_ch = params->client_az[params->client_az_len - 1];
bool is_space =
(first_ch == ' ' || first_ch == '\t' || first_ch == '\n' || first_ch == '\r' ||
first_ch == '\f' || first_ch == '\v' || last_ch == ' ' || last_ch == '\t' ||
last_ch == '\n' || last_ch == '\r' || last_ch == '\f' || last_ch == '\v');
if (is_space) {
const char* az_error_message =
"client_az must not have leading or trailing whitespace.";
VALKEY_LOG_ERROR("valkey_glide_build_client_config_base", az_error_message);
zend_throw_exception(get_valkey_glide_exception_ce(), az_error_message, 0);
return FAILURE;
}
Comment thread
greptile-apps[bot] marked this conversation as resolved.
config->client_az = params->client_az;
Comment thread
prateek-kumar-improving marked this conversation as resolved.
}
}

/* Set lazy connect option */
config->lazy_connect = params->lazy_connect_is_null ? false : params->lazy_connect;
Expand All @@ -241,6 +288,9 @@ int valkey_glide_build_client_config_base(valkey_glide_php_common_constructor_pa
case 3: /* AZ_AFFINITY_REPLICAS_AND_PRIMARY */
config->read_from = VALKEY_GLIDE_READ_FROM_AZ_AFFINITY_REPLICAS_AND_PRIMARY;
break;
case 4: /* AZ_AFFINITY_ALL_NODES */
config->read_from = VALKEY_GLIDE_READ_FROM_AZ_AFFINITY_ALL_NODES;
break;
default: {
const char* error_message = "Invalid read_from value.";
VALKEY_LOG_ERROR("valkey_glide_build_client_config_base", error_message);
Expand All @@ -249,6 +299,33 @@ int valkey_glide_build_client_config_base(valkey_glide_php_common_constructor_pa
}
}

/* AZ affinity read strategies require a client availability zone to be set. */
if (!config->client_az) {
const char* az_error_message = NULL;
switch (config->read_from) {
case VALKEY_GLIDE_READ_FROM_AZ_AFFINITY:
az_error_message = "client_az must be set when read_from is set to AZ_AFFINITY";
break;
case VALKEY_GLIDE_READ_FROM_AZ_AFFINITY_REPLICAS_AND_PRIMARY:
az_error_message =
"client_az must be set when read_from is set to "
"AZ_AFFINITY_REPLICAS_AND_PRIMARY";
break;
case VALKEY_GLIDE_READ_FROM_AZ_AFFINITY_ALL_NODES:
az_error_message =
"client_az must be set when read_from is set to AZ_AFFINITY_ALL_NODES";
break;
default:
break;
}

if (az_error_message) {
VALKEY_LOG_ERROR("valkey_glide_build_client_config_base", az_error_message);
zend_throw_exception(get_valkey_glide_exception_ce(), az_error_message, 0);
return FAILURE;
}
}

/* Map node_discovery_mode enum value to client's NodeDiscoveryMode enum */
switch (params->node_discovery_mode) {
case 0: /* STANDARD */
Expand Down
Loading
Loading