Skip to content
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

Valdation of Redis servers rejects auth parameter #315

Open
stklcode opened this issue Feb 1, 2025 · 0 comments · May be fixed by #317
Open

Valdation of Redis servers rejects auth parameter #315

stklcode opened this issue Feb 1, 2025 · 0 comments · May be fixed by #317

Comments

@stklcode
Copy link
Contributor

stklcode commented Feb 1, 2025

Describe the bug

Redis server configuration rejects valid auth parameter.

To Reproduce

On a system with Redis support, add a filter for Redis servers with 7th (auth) parameter:

add_filter( 'cachify_redis_servers', fn () => [
    '127.0.0.1', 6379, 1, null, 0, 0,
    [ 'auth' => [ 'username', 'password' ] ]
] );

Validation rejects the last parameter, because the condition is invalid:

Internally: Cachify_REDIS::sanitize_con_parameters() returns false because of

if ( count( $con ) > 6 && ! is_null( $con[6] ) && is_array( $con[6] ) ) {
//                                             ^^^^^^^^
	return false;  // Context parameters, e.g. authentication (since PhpRedis 5.3).
}

Expected behavior

Cachify should try to connect to the configured Redis server using "username:password" credentals.

I.e. Cachify_REDIS::sanitize_con_parameters() should return the (sanitized) array.

System:

  • Cachify 2.4.0

Additional context

Cross reference: https://wordpress.org/support/topic/php-fatal-error-457

@stklcode stklcode added this to the 2.4.1 milestone Feb 1, 2025
stklcode added a commit that referenced this issue Feb 1, 2025
The optional 7th argument the Redis::connect() holds an array of context
parameters, required for authentication. This parameter can either be
NULL or an array.

Fix the condition, so it does not reject arrays.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant