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

stay connected checkbox - Add cookie #115

Closed
nicosomb opened this issue Aug 9, 2013 · 58 comments
Closed

stay connected checkbox - Add cookie #115

nicosomb opened this issue Aug 9, 2013 · 58 comments
Assignees
Milestone

Comments

@nicosomb
Copy link
Member

nicosomb commented Aug 9, 2013

pas sur que ça fonctionne. à tester.

@nicosomb
Copy link
Member Author

@ghost ghost assigned nicosomb and tcitworld Sep 16, 2013
@nicosomb
Copy link
Member Author

@tcitworld intéressé pour regarder ce bug ?

@nicosomb
Copy link
Member Author

je viens de mettre à jour la classe Session (http://tontof.net/kriss/php5/session).

quelqu'un peut tester avec la branche de dev please ?

@tcitworld
Copy link
Member

Oui, je regarde cela, ça me semble solvable plus facilement que les histoires de cache.

@nicosomb
Copy link
Member Author

je crois que ça doit être bon en fait ... regarde la branche de dev et dis moi quoi.

@tcitworld
Copy link
Member

Yep, je te dis ça tout de suite.

@tcitworld
Copy link
Member

Hum, non il ne me semble pas que ça fonctionne. J'ai juste viré les données de session mais gardé les cookies et je suis quand même déconnecté.

@tcitworld
Copy link
Member

Autant pour moi, y a pas de système de cookies du tout en fait.

@nicosomb
Copy link
Member Author

Bizarre, j'avais testé avec Chromium tout à l'heure et ça me semblait mieux ... Bon, faudra encore corriger ça.

@nicosomb
Copy link
Member Author

Can you re-test please? I made some changes today.

@tcitworld
Copy link
Member

It doesn't seems so. I'll see the code this week-end.

@nicosomb
Copy link
Member Author

It seems to be fixed: a0aa150

@tcitworld
Copy link
Member

Can't believe I've missed this. So obvious. Well, at least it's done.

@nicosomb nicosomb reopened this Jan 21, 2014
@nicosomb nicosomb mentioned this issue Jan 21, 2014
@jancborchardt
Copy link

cc'ing myself for updates :)

@jancborchardt
Copy link

And copying the stuff I wrote in #401:

One of the most annoying things with both the web interface and especially the browser extensions is that the user log in is very badly remembered. Because of that I continuously need to re-login. For almost every page I want to poche, the app forgot who I am and I need to log in again which makes it very cumbersome.

Instead, on one device, I should only ever need to log in once and everything should be remembered. Until I log out (if I ever do …) of course.

@nicosomb
Copy link
Member Author

It's not a bug, it's a feature.

We use a Session class (http://tontof.net/kriss/php5/session) which don't use cookie.

So, cookie or not for wallabag?

@nicosomb nicosomb added Feature and removed Bug labels Feb 12, 2014
@nicosomb
Copy link
Member Author

I do another try after deleting all my cookies.

@mariroz
Copy link
Contributor

mariroz commented Mar 18, 2014

There are 2 aspects of application and server work flow related to this task:

  1. application level: application sets session cookie with appropriate lifetime. This is done in wallabag now: we can be sure, that browser will receive and don't delete our session during requested time (3 month, if "stay connected" is checked).
  2. server level. Here we can't guarantee anything as can't configure client's server. Each session has an appropriate entry stored on the server. And here is the problem. Default session handler is "file" and here various settings come into play. Important are:
    session.gc_probability, session.gc_divisor and session.gc_maxlifetime. So, if you want to be sure, that your session isn't deleted on server side, pls check these parameters in your php.ini or virtual host config. BUT even this will not guarantee, that your session will work as you expect, if you run in shared environment and session folder is shared between several projects.

According to above, @nicosomb , what do you think: maybe will be good to write some small help file about server config if sessions are closed unexpectedly? I find, that lines like ini_set('session.gc_maxlifetime',6000) in the code are bad practice as will not necessary solve the problem and will try to take unnecessary responsibility. Or we should try to go this way? What do you think?

@nicosomb
Copy link
Member Author

If we can avoid bad pratice, it's better (we have enough bad pratices in wallabag...).
A help file can be good, I'm currently working on a Help page available in the menu.

But, what about people with shared hosting?

mariroz added a commit to mariroz/wallabag that referenced this issue Mar 18, 2014
@mariroz
Copy link
Contributor

mariroz commented Mar 18, 2014

this last commit, however, may cause some problems in some environments.
For example, if session handler is memecache: https://bugs.php.net/bug.php?id=45871

And, of course, it will not fix a problem in shared environament when different applications store it's session data in one directory: http://www.php.net/manual/en/session.configuration.php#ini.session.gc-maxlifetime - session max live time will be minimum value.

I think, will write some howto file.

@mariroz
Copy link
Contributor

mariroz commented Mar 18, 2014

But, what about people with shared hosting?

shred hosting itself is not a problem.
Shared hosting usually is configured to store session files separately for every hosting plan owner/virtual host.
Problem exists, if you run several php applications in the same environment: every application may try to set (or not set) it's own session maxlifetime and this is the problem because min value will work for all. For example here http://blog.centresource.com/2006/05/23/php-session-lifetime-an-adventure/ guy wrote about joomla as a reason of small maxlifetime for other scripts.
Such problems can be fixed only be separate session save_path, but this, in turn, may cause much more serious problems with application security.
It's also not mentioned here, that some settings can be set as denied to override in global config: this also may cause warnings, problems etc.

nicosomb added a commit that referenced this issue Mar 18, 2014
fix of #115, server relater config value added
@nicosomb
Copy link
Member Author

Yeaaah ! It works for me :) !!

@bobmaerten
Copy link
Contributor

Still have this error message with my docker/nginx/php-fpm default config:

10 FastCGI sent in stderr: "PHP message: PHP Notice:  Undefined variable: _SESSION in /var/www/wallabag/inc/3rdparty/class.messages.php on line 62
PHP message: PHP Warning:  array_key_exists() expects parameter 2 to be array, null given in /var/www/wallabag/inc/3rdparty/class.messages.php on line 62"

I have this since removal of session_start(); in index.php in the dev branch, but the trick doesn't work anymore. Shall I add something in my PHP conf ?

@mariroz
Copy link
Contributor

mariroz commented Mar 19, 2014

Hi @bobmaerten , yes, you can add session.auto_start = 1 to your php.ini, or just try my last fix mariroz@6fa3f70 now or after it will be merged by @nicosomb .
Hope, this will help.

@bobmaerten
Copy link
Contributor

Dammit, still doesn't work. Either with your patch, nor session.auto_start = 1, nor both.
I'm cursed. :,(

@mariroz
Copy link
Contributor

mariroz commented Mar 20, 2014

@bobmaerten , n.p. - will try to repeat and fix, I think, tomorrow.

@bobmaerten
Copy link
Contributor

It's probably my setup though. If it works on your configs, it's fine, I'll adapt the docker image to match.

@mariroz
Copy link
Contributor

mariroz commented Mar 21, 2014

@bobmaerten, I repeated you error and hope, now it's fixed. Could you pls try updated dev branch once more?

@bobmaerten
Copy link
Contributor

Just tried again, and still not functionnal. Switched back on master branch to be sure, and it work.
Config just seems not want to run on my setup, and I'm desperate. No error on log this time, and unable to help debugging since my PHP skill are near 0 now.

Is it working on framabag-like config? could you list it, BTW?

@mariroz
Copy link
Contributor

mariroz commented Mar 21, 2014

@bobmaerten, could you pls let me know a bit more about your php configuration: I need phpinfo() output, if possible.

@bobmaerten
Copy link
Contributor

Sure, here it is.

Maybe this will help!

Le ven. 21 mars 2014 16:54:51 CET, mariroz a écrit :

@bobmaerten https://github.com/bobmaerten, could you pls let me know
a bit more about your php configuration: I need phpinfo() output, if
possible.


Reply to this email directly or view it on GitHub
#115 (comment).

@mariroz
Copy link
Contributor

mariroz commented Mar 21, 2014

@bobmaerten , phpinfo not received (maybe, attachment not accepted by github?), could you pls try another way.

@bobmaerten
Copy link
Contributor

Sorry, I thought of that right after sending reply mail. here is a link:

https://owncloud.univ-lille3.fr/public.php?service=files&t=ccae623cc108de4e4ab0807bac3202b3

@mariroz
Copy link
Contributor

mariroz commented Mar 21, 2014

@bobmaerten, could you pls try my last fix, see pull request #582.

@bobmaerten
Copy link
Contributor

\o/ Hurray seems to work as expected! Long shot for a localhost/0.0.0.0 problem.

@mariroz
Copy link
Contributor

mariroz commented Mar 22, 2014

:) glad to hear it.

@nicosomb
Copy link
Member Author

Can I close this issue ? \o/

@nicosomb nicosomb added the Bug label Mar 24, 2014
@bobmaerten
Copy link
Contributor

👍 don't forget to merge #582 :)

@nicosomb
Copy link
Member Author

Done two hours ago ;-)

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

No branches or pull requests

5 participants