View Issue Details

IDProjectCategoryView StatusLast Update
13982Bug reports[All Projects] Survey designpublic2018-09-14 17:12
ReportercaseylucasAssigned ToShnoulleT 
PrioritynoneSeveritymajor 
Status newResolutionfixed 
Product Version3.13.x 
Target VersionFixed in Version 
Summary13982: Uploading survey resources (via kcfinder) does not work with CRedisCache
Description

kcfinder does not work when using CRedisCache because the code in bootstrap.php only conditionally checks for the use of DBHttpSession instead of generally supporting CHttpSession derived classes such as CCacheHttpSession. Reference:
https://github.com/LimeSurvey/LimeSurvey/blob/master/third_party/kcfinder/core/bootstrap.php#L366

Upon failure a popup with the message "You don't have permissions to browse server." is shown when navigating to the "Resources" section of survey in survey settings.

I was able to change the code in checkLSSession (in bootstrap.php) to load configs, call Yii::createAppliation, and call Yii::app()->session->open(). Note the changed code does not create new LSSessionSaveHandler. With this change, I was able to use kcfinder with CRedisCache. I think that with my changes, the LSSessionSaveHandler is redundant because the solution should work with any CHttpSession derived class which returns true from 'getUseCustomStorage'. kcfinder requires a call to 'session_set_save_handler' and CHttpSession makes this call when 'getUseCustomStorage' returns true:
https://github.com/LimeSurvey/LimeSurvey/blob/master/framework/web/CHttpSession.php#L112

So, I just want to make sure that my approach is sane before I clean up the code and submit a PR which would eliminate some of the code in bootstrap.php. Idea taken from https://www.yiiframework.com/wiki/421/syncing-sessions-between-yii-and-kcfinder#c19588

Thoughts?

Steps To Reproduce

My relevant config sections:

    'cache'=>array(
        'class'=>'CRedisCache',
        'hostname' => 'redis',
        'port'=>6379,
        'database'=>0,
        'options'=>STREAM_CLIENT_CONNECT,
    ),
    'session' => array(
        'class'=>'CCacheHttpSession',
        'cookieParams' => array(
            'httponly' => true,
            'secure' => (
                (!empty($_SERVER['HTTP_X_SCHEME']) && $_SERVER['HTTP_X_SCHEME'] == 'https') ||
                (!empty($_SERVER['HTTPS']) && $_SERVER['HTTPS'] !== 'off') ||
                (isset($_SERVER['SERVER_PORT']) && $_SERVER['SERVER_PORT'] == 443)
            )
        ),
        # timeout sessions in 30 minutes
        'timeout' => 1800,
    ),
Additional Information

Possibly related:
https://bugs.limesurvey.org/view.php?id=13703
https://bugs.limesurvey.org/view.php?id=13503

TagsNo tags attached.
Complete LimeSurvey version number (& build)3.13.2+180709
I will donate to the project if issue is resolvedNo
Browserany
Database & DB-VersionPostgres
Operating System (Server)linux
Webserver software & versionFPM
PHP Version7.2

Activities

caseylucas

caseylucas

2018-08-20 22:20

reporter   ~48832

I created a PR so that ideas and code can be reviewed: https://github.com/LimeSurvey/LimeSurvey/pull/1112

I also confirmed that the solution works with DBHttpSession.

LouisGac

LouisGac

2018-08-21 11:36

manager   ~48837

CRedisCache is not supported by LimeSurvey. I don't think we will put any effort in it for now.
It seems to be supported by Yii1 though.
So you can try to update your DB configuration in config.php to see if it fixes some problems:
https://www.yiiframework.com/doc/api/1.1/CRedisCache

caseylucas

caseylucas

2018-08-21 15:46

reporter   ~48842

Thanks Louis. The solution in the PR works with CRedisCache, existing DBHttpSession, and should work with any CHttpSession derived class. Is there any reason that the more general solution should not be preferred? Maybe there is a different preferred strategy?

By the way, I have CRedisCache working fine for other aspects of sessioning. It's only the kcfinder integration that I am currently having issues. The PR fixes the issue but I was hoping to get an expert's opinion on the strategy.

LouisGac

LouisGac

2018-08-22 12:21

manager   ~48848

I've seen your PR. We'll merge it after a review and some testing.

DenisChenu

DenisChenu

2018-08-22 12:26

developer   ~48849

@LouisGac : if Yii1 accept redis cache : LS can accept Redis cache.

I think the 2 PR are really great and can be merged https://github.com/LimeSurvey/LimeSurvey/pull/1111 + https://github.com/LimeSurvey/LimeSurvey/pull/1112

caseylucas

caseylucas

2018-09-13 17:55

reporter   ~49027

Any update on this issue? Anything I can do to help?

ShnoulleT

ShnoulleT

2018-09-14 09:03

reporter   ~49031

Fix committed to master branch: http://bugs.limesurvey.org/plugin.php?page=Source/view&id=27949

caseylucas

caseylucas

2018-09-14 17:12

reporter   ~49046

It's certainly possible that I'm missing something but I think there may have been a mixup. PR https://github.com/LimeSurvey/LimeSurvey/pull/1111 (fix for bug 13888) has been merged but PR https://github.com/LimeSurvey/LimeSurvey/pull/1112 which is a fix for this issue has not been merged.

Issue History

Date Modified Username Field Change
2018-08-20 21:08 caseylucas New Issue
2018-08-20 22:20 caseylucas Note Added: 48832
2018-08-21 11:36 LouisGac Note Added: 48837
2018-08-21 15:46 caseylucas Note Added: 48842
2018-08-22 12:21 LouisGac Note Added: 48848
2018-08-22 12:26 DenisChenu Note Added: 48849
2018-09-13 17:55 caseylucas Note Added: 49027
2018-09-14 09:03 ShnoulleT Changeset attached => LimeSurvey master c0ef15f5
2018-09-14 09:03 ShnoulleT Note Added: 49031
2018-09-14 09:03 ShnoulleT Assigned To => ShnoulleT
2018-09-14 09:03 ShnoulleT Resolution open => fixed
2018-09-14 09:06 DenisChenu Changeset removed LimeSurvey master c0ef15f5 =>
2018-09-14 17:12 caseylucas Note Added: 49046