zendframework / zend-session

Manage and preserve session data, a logical complement of cookie data, across multiple page requests by the same client.
BSD 3-Clause "New" or "Revised" License
42 stars 62 forks source link

Redis as save handler #98

Closed ikovalyov closed 6 years ago

ikovalyov commented 6 years ago

Provide a narrative description of what you are trying to accomplish.

Code to reproduce the issue

                $config->setOption("save_handler", "redis");
                $url = "tcp://" . $redisDto->getHost() . ":" . $redisDto->getPort() . "?auth=" . $redisDto->getPassword() . "&database=1";
                $config->setOption("save_path", $url);

Expected results

It should save data in redis

Actual results

Warning: sessionstart(): open(tcp://redis:?auth=&database=1/sess, O_RDWR) failed: No such file or directory (2) in /vendor/zendframework/zend-session/src/SessionManager.php on line 140

Here is what I found in your code:

    public function setStorageOption($storageName, $storageValue)
    {
        switch ($storageName) {
            case 'remember_me_seconds':
                // do nothing; not an INI option
                return;
            case 'url_rewriter_tags':
                $key = 'url_rewriter.tags';
                break;
            case 'save_handler':
                // Save handlers must be treated differently due to changes
                // introduced in PHP 7.2. Do not alter running INI setting.
                return $this;
            default:
                $key = 'session.' . $storageName;
                break;
        }

The 2.8.0 version looks like

    public function setStorageOption($storageName, $storageValue)
    {
        switch ($storageName) {
            case 'remember_me_seconds':
                // do nothing; not an INI option
                return;
            case 'url_rewriter_tags':
                $key = 'url_rewriter.tags';
                break;
            default:
                $key = 'session.' . $storageName;
                break;
        }

If I pass redis as save_handler it's just ignored and php trying to use my redis config as default config parameters. It broken my live servers. Reverted to 2.8.0 for now.

Please fix ASAP.

Thanks.

weierophinney commented 6 years ago

Fixed with #99 and released with 2.8.2.

ikovalyov commented 6 years ago

Actually it wasn't fixed

v 2.8.2:

Warning: session_start(): open(tcp://redis:6379?auth=password&database=1/sess_139bc23a33fc5fdda65bc4b7b9268c69, O_RDWR) failed: No such file or directory (2) in /opt/webapp/***/vendor/zendframework/zend-session/src/SessionManager.php on line 140

Warning: session_start(): Failed to read session data: files (path: tcp://redis:6379?auth=password&database=1) in /opt/webapp/***/vendor/zendframework/zend-session/src/SessionManager.php on line 140

v 2.8.0 everything works correctly. code looks like

                $config->setOption("save_handler", "redis");
                $url = "tcp://" . $redisDto->getHost() . ":" . $redisDto->getPort() . "?auth=" . $redisDto->getPassword() . "&database=1";
                $config->setOption("save_path", $url);
ikovalyov commented 6 years ago

@weierophinney

weierophinney commented 6 years ago

@ikovalyov — Your diagnosis is incorrect.

The change in behavior of setStorageOption() is correct; if that change is not present, then the code would call ini_set('session.save_handler', $value), which causes issues within PHP 7.2 in a number of cases.

What we are doing internally instead is, within setPhpSaveHandler(), calling session_module_name($saveHandler) for built-in save handlers, and session_set_save_handler() for any custom save handlers. getStorageOption() has been updated to return the value of session_module_name() unless a custom save handler was used (in which case, the class name is returned).

I'm going to write a unit test based on your example above, and debug from there; I'll link this to a new pull request once I have a solution ready to review.

ikovalyov commented 6 years ago

thanks