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

Upgrade of php5 from 5.4.45-0+deb7u7 to 5.4.45-0+deb7u8 on Debian causes redis connection error #111

Open
ciderpunx opened this issue Apr 13, 2017 · 8 comments

Comments

@ciderpunx
Copy link

We recently upgraded php5 on our Debian Wheezy server to a more recent version. After the upgrade Magento started crashing with these errors:

a:5:{i:0;s:126:"Connection to Redis 127.0.0.1:6379 failed after 2 failures.Last Error : (0) Failed to parse address "127.0.0.1:6379/cache-fpc"";i:1;s:3217:"#0 /path/to/magento/.modman/Cm_RedisSession/cr
    edis/Client.php(448): Credis_Client->connect()

Nothing else has changed.

Currently I have php held at the older version which resolves the problem.

Is there something I need to do to make the newer php play nice (I'd like to start moving towards using stretch this year, so using later php versions is important for me).

@colinmollenhour
Copy link
Owner

Ahh actually it looks like another user just reported the issue with the explanation on colinmollenhour/credis#79..

colinmollenhour referenced this issue in php/php-src Apr 13, 2017
For historical reasons, fsockopen() accepts the port and hostname
separately: fsockopen('127.0.0.1', 80)

However, with the introdcution of stream transports in PHP 4.3,
it became possible to include the port in the hostname specifier:

fsockopen('127.0.0.1:80')
Or more formally: fsockopen('tcp://127.0.0.1:80')

Confusing results when these two forms are combined, however.
fsockopen('127.0.0.1:80', 443) results in fsockopen() attempting
to connect to '127.0.0.1:80:443' which any reasonable stack would
consider invalid.

Unfortunately, PHP parses the address looking for the first colon
(with special handling for IPv6, don't worry) and calls atoi()
from there.  atoi() in turn, simply stops parsing at the first
non-numeric character and returns the value so far.

The end result is that the explicitly supplied port is treated
as ignored garbage, rather than producing an error.

This diff replaces atoi() with strtol() and inspects the
stop character.  If additional "garbage" of any kind is found,
it fails and returns an error.
@colinmollenhour
Copy link
Owner

Quick fix is to remove your persistent connection identifier from the connection string in your config. If you use the same Redis server for cache and sessions and have persistent connections enabled for both with different ids this could be a problem.. For now I think you would need to disable persistent connections entirely to avoid the same connection being used for cache and sessions, although I haven't tested this to confirm.

@ciderpunx
Copy link
Author

Thanks for your prompt response and many apologies for my tardy reply. Easter and all that.

As it happened, changing my config in local.xml to have empty strings for persistent did not resolve the issue.

I did however manage to fix it by commenting line 430 which adds the identifier in Cm_RedisSession/credis/Client.php:

 428             if ($this->persistent && $this->port !== NULL) {
 429                 // Persistent connections to UNIX sockets are not supported
 430                 // $remote_socket .= '/'.$this->persistent;
 433                 $flags = $flags | STREAM_CLIENT_PERSISTENT;
 434             }

@colinmollenhour
Copy link
Owner

Are you using the latest version? It looks like in the latest version the persistent option is cast to a string so a patch should not be necessary. If it was not cast in an older version then it could be evaluating to a truthy value (xml node instance).

@ciderpunx
Copy link
Author

I think so but don't know where to look for the version number!

$ modman update Cm_RedisSession
Already up-to-date.
Submodule 'php-redis-session-abstract' () registered for path 'code/lib'
Submodule 'credis' () registered for path 'credis'
Update of Cm_RedisSession complete.

I can see that persistent gets cast on line 303, so it points to being the right version:
303 $this->persistent = (string) $persistent;

@colinmollenhour
Copy link
Owner

Ok, I didn't know if you had the bundled version or the github module. If you just cloned it off of github with modman then you should be using the latest by default.

Have you tried commenting out the <persistent> node in the config instead of specifying an empty string? It really shouldn't make a difference but I have not had the opportunity to look into it further. Please post the <redis_session> node of your config if you are still having issues.

@jefweb
Copy link

jefweb commented Apr 20, 2017

@colinmollenhour I have tested this briefly and can confirm that commenting out the <persistent> node allowed a successful connection. If using Redis for caching will have to do this on the <cache> and <full_page_cache> nodes as well.

@Amadeco
Copy link

Amadeco commented Feb 28, 2018

Thank you for the solution !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants