Add supoort to Redis Cluster password，and add auth in php.ini

[andyli029]

Add supoort to Redis Cluster password, but not support Redis Cluster password added in php.ini temporarily

[JacketWoo]

Our team currently also needs this function. I have test your code，generally it works，wakaka. But I have two little suggestions: 1）using 'estrdup' to get a copy of auth string instead of using ecalloc and memcpy, otherwise, it may results in error in 'cmd_len = redis_cmd_format_static(&cmd, "AUTH", "s", redis_sock->auth, strlen(redis_sock->auth));' sentence of resend_auth procedure(strlen(redis_sock->auth) may be greater than original auth string's length); 2) return NULL instead of -1 in the first 'if' branch of 'cluster_get_slots' procedure may be better.
@michael-grunder Do you plans to add this function yourself, or merge this code? We are looking forward to it.

[waxdsf]

I was a novice for redis and now I am tring to use redis cluster with password. Thanks for your code, but I find no example to use your code, can you teach me how to connect to redis cluster with password in PHP? c is very difficult for me. @andyli029

[michael-grunder]

@waxdsf I'm not the author of this pull request, but from looking at the code it appears that it adds an argument which will come right after your seeds array, which is just a string (the password).

[waxdsf]

@michael-grunder Thanks for your answer. I forget to restart php-fpm so that when I second install phpredis, so it does not work with password. I made a stupid mistake, it works well now. Anyway, thank you!

[tillkruss]

How can I set a password for a cluster?

[tillkruss]

Is it possible to specify in the seed string?

[andyli029]

the simple example:

<?php
   $credis = new RedisCluster(NULL, Array('10.150.110.172:6005',),"9MEL1CDSYY");
   //$credis = new RedisCluster('mycluster');
   var_dump($credis);
   echo "Connection to server sucessfully\n";
   $credis->set('q', 'wwww');
   echo "Server is running: " . $credis->get('q') . "\n" ;
?>

the password is “9MEL1CDSYY”

[tillkruss]

Ah, so the method signature changes from:

new RedisCluster(null, $seeds, $timeout, $readTimeout, $persistent)

to the new one:

new RedisCluster(null, $seeds, $password, $timeout, $readTimeout, $persistent)

Correct?

[andyli029]

You are right!
The new is
new RedisCluster(null, $seeds, $timeout, $readTimeout, $persistent, $password)

[andyli029]

@JacketWoo @yatsukhnenko Thanks for your suggestions.
I have use estrndup instead of ecalloc+memcpy.
@yatsukhnenko,please merge the commit.

[yatsukhnenko]

@andyli029, I'm not really familiar with cluster so it will be better if @michael-grunder will review/merge

[michael-grunder]

@andyli029 I'll test it locally but it looks good on first glance. The only issue I see is that it changes the argument order of RedisCluster__construct which will break people's code.

[andyli029]

@michael-grunder
what is your mean in PHP_METHOD(RedisCluster, __construct) ：

     if(zend_parse_method_parameters(ZEND_NUM_ARGS() TSRMLS_CC, getThis(),
                                   "Os|asddb", &object, redis_cluster_ce, &name,
                                   &name_len, &z_seeds, &auth, &auth_len, &timeout,
                                   &read_timeout, &persistent)==FAILURE)

is changed to

     if(zend_parse_method_parameters(ZEND_NUM_ARGS() TSRMLS_CC, getThis(),
                                    "Os|addbs", &object, redis_cluster_ce, &name,
                                    &name_len, &z_seeds, &timeout,
                                    &read_timeout, &persistent,
                                    &auth, &auth_len)==FAILURE)

or how to modify？

[michael-grunder]

Aha, I must have been looking at an old version

On Mon, Aug 22, 2016, 10:59 PM andyli notifications@github.com wrote:

@michael-grunder https://github.com/michael-grunder
what is your mean：

 if(zend_parse_method_parameters(ZEND_NUM_ARGS() TSRMLS_CC, getThis(),
                               "Os|asddb", &object, redis_cluster_ce, &name,
                               &name_len, &z_seeds, &auth, &auth_len, &timeout,
                               &read_timeout, &persistent)==FAILURE)

changed to
if(zend_parse_method_parameters(ZEND_NUM_ARGS() TSRMLS_CC, getThis(),
"Os|addbs", &object, redis_cluster_ce, &name,
&name_len, &z_seeds, &timeout,
&read_timeout, &persistent,
&auth, &auth_len)==FAILURE)

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#902 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAcktR0cpTNC1wchArAs3Md-L9VqJ39Qks5qioxCgaJpZM4JTfHw
.

[andyli029]

@michael-grunder please review and merge the commit.

[bartlomiejsawicki]

When it will be merged? I really would like to have this feature in your official release.

[bradcornford]

@michael-grunder Any ideas when this PR will be merged?

[yatsukhnenko]

Closing in favour to #1363