Hello!
I'm trying to ignore all the rants and personal accusations and get back to the a constructive conversation about a technical problem. First, let's start a new thread since it has actually nothing to do with the 0.9.1 update.
Second, there are reasons that Roundcube comes with a custom session handler and those are still valid. We didn't do this just for fun, believe me!
The main reason are concurrent http request which alter session data. Image the following scenario:
A) [r]----------- big file being uploaded ------------[w] B) [r]----- fast upload ----[w]
In PHP, session data is read when the process starts and written when the process ends. With the scenario from above, the session changes of request B) would be lost when A) finishes. Until now I don't know of any solution with default PHP session handlers that would be able to handle such a case properly. Another more performance related reason for you custom handler is the check if session data actually changed before issuing a (useless) update query.
Sure, we could store file upload information somewhere else than in session but the underlying problem of concurrent requests overlapping each others still persists and can happen in other cases, too.
On Wed, May 22, 2013 at 9:00 PM, till klimpong@gmail.com wrote:
I agree that session handling could be nicer in RoundCube — or maybe not at all. ;-) If we wouldn't have customized session handling code we could let people configure PHP directly to use the filesystem, memcache or database. I think the least we could (or should) do is use an external library for session handling because that doesn't seem our domain (which is an email client).
Not sure if we could use something from Symfony2 or ZF2. They each provide wrappers — PHP 5.3, et all. Are people ready to move/upgrade yet?
We're open for suggestions to get rid of the custom session handlers but please keep the above scenario in mind. Switching to an external library might be a solution but don't they use custom save handlers, too?
Speaking of which: what Roundcube does is to call session_set_save_handler() in a very early stage of the script startup. That's the API PHP provides and we sticked to the documentation for implementing the handler functions including returning true to signalize success. Unfortunately I don't have an explanation for the errors you reported without being able to reproduce it. So let's try it the old fashioned way and eliminate possible factors. @Harald, could you run Roundcube on a PHP5.4 without Suhosin to rule that out first?
And please everybody, choose your words wisely!
Regards, Thomas
-------- Original-Nachricht -------- Betreff: Re: [RCD] Update 0.9.1 released Datum: Mon, 20 May 2013 16:39:51 +0200 Von: Reindl Harald h.reindl@thelounge.net
and when will sessions continue to work on machines with Apache 2.4 / PHP 5.4 / MariaDB while the error messages below make ZERO sense because RC refuses to work with a untouched session management like every other webapp
would RC use simply session_start() and not fuckup the admin settings it *would* in fact use /var/www/sessiondata and it would it use *with success*
** give us a option to NOT touch any session setting **
P.S. we can but for the just explained reasons your Roundcube users will complain about lost attachments and other weird behavior.
Am 26.05.2013 17:59, schrieb Thomas Bruederli:
Second, there are reasons that Roundcube comes with a custom session handler and those are still valid. We didn't do this just for fun, believe me!
i did not say this, but they are not relevant in all setups
The main reason are concurrent http request which alter session data. Image the following scenario:
A) [r]----------- big file being uploaded ------------[w] B) [r]----- fast upload ----[w]
In PHP, session data is read when the process starts and written when the process ends. With the scenario from above, the session changes of request B) would be lost when A) finishes. Until now I don't know of any solution with default PHP session handlers that would be able to handle such a case properly
and that is why session-files are locked
well, you could came now again with performance but i am developing long enough wep-applications with real load to not see why RC is special
Another more performance related reason for you custom handler is the check if session data actually changed before issuing a (useless) update query
without the custom handler there would be no db query at all
Sure, we could store file upload information somewhere else than in session but the underlying problem of concurrent requests overlapping each others still persists and can happen in other cases, too
if it comes to integrity it is a *bad idea* write *lockless* in sessiondata which does not happen in the default handler
@Harald, could you run Roundcube on a PHP5.4 without Suhosin to rule that out first?
it is the combination of Suhosin and PHP 5.4
but since my main-job is php-developer and all applications i developed and maintain except RC are working in the test-setups and on all of the developer-machines i have zero understanding for *one* app which does not and there are much more critical things like a webmail client which is nice-to-have but not more if it comes to business users
** give us a option to NOT touch any session setting **
P.S. we can but for the just explained reasons your Roundcube users will complain about lost attachments and other weird behavior
why does this need a *custom* session handler? sorry - no, zero understanding
Reindl Harald wrote:
Am 26.05.2013 17:59, schrieb Thomas Bruederli:
Second, there are reasons that Roundcube comes with a custom session handler and those are still valid. We didn't do this just for fun, believe me!
i did not say this, but they are not relevant in all setups
The main reason are concurrent http request which alter session data. Image the following scenario:
A) [r]----------- big file being uploaded ------------[w] B) [r]----- fast upload ----[w]
In PHP, session data is read when the process starts and written when the process ends. With the scenario from above, the session changes of request B) would be lost when A) finishes. Until now I don't know of any solution with default PHP session handlers that would be able to handle such a case properly
and that is why session-files are locked
well, you could came now again with performance but i am developing long enough wep-applications with real load to not see why RC is special
Will PHP just sleep B) until session-files are unlocked or will it just fail in writing the session data? And do database backends of PHPs session handler do locking as well?
Maybe my knowledge about that is outdated and we can indeed drop our custom handlers but that all wasn't given some years ago when we ran into problems with the default PHP session storage. And if PHP offers the ability to implement custom handlers you shouldn't blame the ones who make use of that feature.
And for scalability reasons we should make sure *all* possible PHP session backends behave the same and not only talk about file-based session storage.
Another more performance related reason for you custom handler is the check if session data actually changed before issuing a (useless) update query
without the custom handler there would be no db query at all
I don't get that. AFAIK PHP always writes session data on script shutdown even if it didn't change. Using a database backend thus will trigger useless UPDATE queries. But let's put that aside since that's not the main reason for our custom handler anyways.
Sure, we could store file upload information somewhere else than in session but the underlying problem of concurrent requests overlapping each others still persists and can happen in other cases, too
if it comes to integrity it is a *bad idea* write *lockless* in sessiondata which does not happen in the default handler
@Harald, could you run Roundcube on a PHP5.4 without Suhosin to rule that out first?
it is the combination of Suhosin and PHP 5.4
but since my main-job is php-developer and all applications i developed and maintain except RC are working in the test-setups and on all of the developer-machines i have zero understanding for *one* app which does not and there are much more critical things like a webmail client which is nice-to-have but not more if it comes to business users
Shall I take this as *no*?
Fair enough, but if you're not willing to help us tracking the issue down, then you shouldn't expect a solution anytime soon as nobody else is obviously able to reproduce the errors you're reporting.
** give us a option to NOT touch any session setting **
P.S. we can but for the just explained reasons your Roundcube users will complain about lost attachments and other weird behavior
why does this need a *custom* session handler? sorry - no, zero understanding
We had reproducible errors reported where session data was lost due to concurrent requests overwriting each other.
~Thomas
Am 26.05.2013 19:23, schrieb Thomas Bruederli:
Will PHP just sleep B) until session-files are unlocked or will it just fail in writing the session data? And do database backends of PHPs session handler do locking as well?
it sleeps
you can see this clearly if you have a frameset and every frame is using the session and can imporve this if you do sessin_write_close() as soon as you know you will not write changes to the session data
if this is done right you see no delays in loading the frameset
Maybe my knowledge about that is outdated and we can indeed drop our custom handlers but that all wasn't given some years ago when we ran into problems with the default PHP session storage
no need to drop, make it optional and all are happy the ones with shared sessions over load balancers and the one which have not the need because zero load in context of webmail
it should be quite simple to skip the custom session handler
BTW: you should use session_write_close(); if you are working with redirects, that's why i use a custom redirect method which also disconnects the db-connection as soon as possible
public function location($location, $permanent=false) { if(PHP_SAPI == 'cli') { return false; } session_write_close(); $this->db->disconnect(); switch($permanent) { case true: header($_SERVER['SERVER_PROTOCOL'] . ' 301 Moved Permanently', true, 301); exit(header('Location: ' . $location, true, 301)); break; case false: header($_SERVER['SERVER_PROTOCOL'] . ' 302 Moved Temporarily', true, 302); exit(header('Location: ' . $location, true, 302)); break; } }
it is the combination of Suhosin and PHP 5.4
but since my main-job is php-developer and all applications i developed and maintain except RC are working in the test-setups and on all of the developer-machines i have zero understanding for *one* app which does not and there are much more critical things like a webmail client which is nice-to-have but not more if it comes to business users
Shall I take this as *no*? Fair enough, but if you're not willing to help us tracking the issue down
it is *clearly* Suhosin + custom session handlers + PHP 5.4 what i have ztracked down days ago in the list
Reindl Harald wrote:
Am 26.05.2013 19:23, schrieb Thomas Bruederli:
Will PHP just sleep B) until session-files are unlocked or will it just fail in writing the session data? And do database backends of PHPs session handler do locking as well?
it sleeps
And mysql or mongodb handlers, too?
you can see this clearly if you have a frameset and every frame is using the session and can imporve this if you do sessin_write_close() as soon as you know you will not write changes to the session data
if this is done right you see no delays in loading the frameset
Maybe my knowledge about that is outdated and we can indeed drop our custom handlers but that all wasn't given some years ago when we ran into problems with the default PHP session storage
no need to drop, make it optional and all are happy the ones with shared sessions over load balancers and the one which have not the need because zero load in context of webmail
it should be quite simple to skip the custom session handler
Should be, although there's some more functionality and checking attached to it. But I'll look into it.
BTW: you should use session_write_close(); if you are working with redirects, that's why i use a custom redirect method which also disconnects the db-connection as soon as possible
[...]
We registered a shutdown function that triggers session_write_close() before closing the database connection. Should be the same at the end.
it is the combination of Suhosin and PHP 5.4
but since my main-job is php-developer and all applications i developed and maintain except RC are working in the test-setups and on all of the developer-machines i have zero understanding for *one* app which does not and there are much more critical things like a webmail client which is nice-to-have but not more if it comes to business users
Shall I take this as *no*? Fair enough, but if you're not willing to help us tracking the issue down
it is *clearly* Suhosin + custom session handlers + PHP 5.4 what i have ztracked down days ago in the list
Sorry, I seem to have missed that.
~Thomas
Am 26.05.2013 19:50, schrieb Thomas Bruederli:
Reindl Harald wrote:
Am 26.05.2013 19:23, schrieb Thomas Bruederli:
Will PHP just sleep B) until session-files are unlocked or will it just fail in writing the session data? And do database backends of PHPs session handler do locking as well?
it sleeps
And mysql or mongodb handlers, too?
no, it's the job of the handler implement locking are you not doing this currently?
it should be quite simple to skip the custom session handler
Should be, although there's some more functionality and checking attached to it. But I'll look into it.
thanks!
it is *clearly* Suhosin + custom session handlers + PHP 5.4 what i have ztracked down days ago in the list
Sorry, I seem to have missed that
no problem
Reindl Harald wrote:
Am 26.05.2013 19:50, schrieb Thomas Bruederli:
Reindl Harald wrote:
Am 26.05.2013 19:23, schrieb Thomas Bruederli:
Will PHP just sleep B) until session-files are unlocked or will it just fail in writing the session data? And do database backends of PHPs session handler do locking as well?
it sleeps
And mysql or mongodb handlers, too?
no, it's the job of the handler implement locking are you not doing this currently?
it should be quite simple to skip the custom session handler
Should be, although there's some more functionality and checking attached to it. But I'll look into it.
thanks!
Please find attached a patch that adds the option to use the native PHP session save handlers in Roundcube. Could you give it a try before we commit it to git?
Hint: you need to set
$rcmail_config['session_storage'] = 'php';
in your local config/main.inc.php
Am 30.05.2013 12:32, schrieb Thomas Bruederli:
Am 26.05.2013 19:23, schrieb Thomas Bruederli:
Will PHP just sleep B) until session-files are unlocked or will it just fail in writing the session data? And do database backends of PHPs session handler do locking as well?
it sleeps
And mysql or mongodb handlers, too?
no, it's the job of the handler implement locking are you not doing this currently?
it should be quite simple to skip the custom session handler
Should be, although there's some more functionality and checking attached to it. But I'll look into it.
thanks!
Please find attached a patch that adds the option to use the native PHP session save handlers in Roundcube. Could you give it a try before we commit it to git?
Hint: you need to set $rcmail_config['session_storage'] = 'php';
thank you!
i included the patch im my RPM-SPEC, changed the config and the session-table keeps empty while read messages, lists and compose a message with a attachment works, even after RC said the target adress wiht "@test.rh" is wrong while the SMTP has a transport :-)
Reindl Harald wrote:
Am 30.05.2013 12:32, schrieb Thomas Bruederli:
Am 26.05.2013 19:23, schrieb Thomas Bruederli:
Will PHP just sleep B) until session-files are unlocked or will it just fail in writing the session data? And do database backends of PHPs session handler do locking as well?
it sleeps
And mysql or mongodb handlers, too?
no, it's the job of the handler implement locking are you not doing this currently?
it should be quite simple to skip the custom session handler
Should be, although there's some more functionality and checking attached to it. But I'll look into it.
thanks!
Please find attached a patch that adds the option to use the native PHP session save handlers in Roundcube. Could you give it a try before we commit it to git?
Hint: you need to set $rcmail_config['session_storage'] = 'php';
thank you!
i included the patch im my RPM-SPEC, changed the config and the session-table keeps empty while read messages, lists and compose a message with a attachment works, even after RC said the target adress wiht "@test.rh" is wrong while the SMTP has a transport :-)
Thanks for testing! However, the sending issue most likely isn't session related.
I've not committed the patch to git master. Please note that some cleanup jobs within Roundcube are tied to the session garbage collector calls which do not happen when using the native php session save handler. Thus you should call bin/gc.sh periodically with a cron job.
I hope this finally adds the amount of flexibility you're requesting.
~Thomas
Am 05.06.2013 09:15, schrieb Thomas Bruederli:
Reindl Harald wrote:
Am 30.05.2013 12:32, schrieb Thomas Bruederli:
Am 26.05.2013 19:23, schrieb Thomas Bruederli: > Will PHP just sleep B) until session-files are unlocked or will it just > fail in writing the session data? And do database backends of PHPs session > handler do locking as well? it sleeps
And mysql or mongodb handlers, too?
no, it's the job of the handler implement locking are you not doing this currently?
it should be quite simple to skip the custom session handler
Should be, although there's some more functionality and checking attached to it. But I'll look into it.
thanks!
Please find attached a patch that adds the option to use the native PHP session save handlers in Roundcube. Could you give it a try before we commit it to git?
Hint: you need to set $rcmail_config['session_storage'] = 'php';
thank you!
i included the patch im my RPM-SPEC, changed the config and the session-table keeps empty while read messages, lists and compose a message with a attachment works, even after RC said the target adress wiht "@test.rh" is wrong while the SMTP has a transport :-)
Thanks for testing! However, the sending issue most likely isn't session related.
i know, but it shows that the session is stable if the attachment got not lost
I've not committed the patch to git master. Please note that some cleanup jobs within Roundcube are tied to the session garbage collector calls which do not happen when using the native php session save handler. Thus you should call bin/gc.sh periodically with a cron job.
ok, i take a look what this script does while i can not imagine cleanups at the moment except temporary stored attachments
I hope this finally adds the amount of flexibility you're requesting
of course - thank you!
On 05/26/2013 06:27 PM, Reindl Harald wrote:
but since my main-job is php-developer and all applications i developed
Come on. For php programmer it shouldn't take more than an hour to replace Roundcube session handler with PHP's default. You need some small modifications in rcube_session class. Then you can do some performance and usability comparisons to convince as to use/support such solution.
Am 27.05.2013 15:07, schrieb A.L.E.C:
On 05/26/2013 06:27 PM, Reindl Harald wrote:
but since my main-job is php-developer and all applications i developed
Come on. For php programmer it shouldn't take more than an hour to replace Roundcube session handler with PHP's default. You need some small modifications in rcube_session class. Then you can do some performance and usability comparisons to convince as to use/support such solution
and backport this hack with every update...................
On 05/27/2013 03:54 PM, Reindl Harald wrote:
Come on. For php programmer it shouldn't take more than an hour to replace Roundcube session handler with PHP's default. You need some small modifications in rcube_session class. Then you can do some performance and usability comparisons to convince as to use/support such solution
and backport this hack with every update...................
So, you will not work on this to convince us to include such feature? Then we (at least I) will not work to do this for you. EOT.
On Sunday, May 26, 2013 at 5:59 PM, Thomas Bruederli wrote:
Hello!
I'm trying to ignore all the rants and personal accusations and get back to the a constructive conversation about a technical problem. First, let's start a new thread since it has actually nothing to do with the 0.9.1 update.
Second, there are reasons that Roundcube comes with a custom session handler and those are still valid. We didn't do this just for fun, believe me!
The main reason are concurrent http request which alter session data. Image the following scenario:
A) [r]----------- big file being uploaded ------------[w] B) [r]----- fast upload ----[w]
In PHP, session data is read when the process starts and written when the process ends. With the scenario from above, the session changes of request B) would be lost when A) finishes.
I'm not a 100% sure I understand the example. Can point out where this issue bubbles up in RoundCube?
I just haven't heard of this issue either. But I'd like to understand.
It doesn't sound like databasing the sessions would be a solution either if one process wipes out another process' data.
Until now I don't know of any solution with default PHP session handlers that would be able to handle such a case properly. Another more performance related reason for you custom handler is the check if session data actually changed before issuing a (useless) update query.
Sure, we could store file upload information somewhere else than in session but the underlying problem of concurrent requests overlapping each others still persists and can happen in other cases, too.
As far as I understand, what happens in a request, stays in a request. I don't see how one request would override another.
The backend shouldn't matter. Databased sessions can have some advantages as they make scaling infrastructure easier but of course they could be considered overhead in a single-server setup.
I think the backend (file storage, database, memcache, redis) etc. should not have an impact unless we delete all data and re-save it every time. But anyway, let me know if you can explain this in more details.
Maybe this is an artifact from PHP4 times?
On Wed, May 22, 2013 at 9:00 PM, till <klimpong@gmail.com (mailto:klimpong@gmail.com)> wrote:
I agree that session handling could be nicer in RoundCube — or maybe not at all. ;-) If we wouldn't have customized session handling code we could let people configure PHP directly to use the filesystem, memcache or database. I think the least we could (or should) do is use an external library for session handling because that doesn't seem our domain (which is an email client).
Not sure if we could use something from Symfony2 or ZF2. They each provide wrappers — PHP 5.3, et all. Are people ready to move/upgrade yet?
We're open for suggestions to get rid of the custom session handlers but please keep the above scenario in mind. Switching to an external library might be a solution but don't they use custom save handlers, too?
They do — but RoundCube's session handling does something else which makes it incompatible with Suhosin. I've been running various Symfony2 (actually Silex) and ZF1 projects with their session handling code and Suhosin enabled — no issues.
But I also agree that this is not a new issue. It's why we suggested people turn off certain Suhosin options (see installer checks).
Till
On 05/27/2013 02:20 PM, till wrote:
As far as I understand, what happens in a request, stays in a request. I don't see how one request would override another.
Consider two requests:
- [R1] start file upload
- [R2] start file upload
- [R2] finish upload, save file info into $_SESSION['uploads'], exit
- [R1] finish upload, save file info into $_SESSION['uploads'], exit
In such case:
- Will R1 overwrite $_SESSION['uploads'] set by R2? Yes. This however
I'm not sure now. It's also possible that because of session locking R2 will not finish before R1. 2. Will it lock session file while uploading which makes impossible to show upload progress?
They do — but RoundCube's session handling does something else which makes it incompatible with Suhosin.
This is known suhosin issue and I posted to the list already a link to PR that fixes it. So, please stop digging suhosin issue here.
https://github.com/stefanesser/suhosin/pull/26
On Mon, May 27, 2013 at 2:20 PM, till klimpong@gmail.com wrote:
On Sunday, May 26, 2013 at 5:59 PM, Thomas Bruederli wrote:
Hello!
I'm trying to ignore all the rants and personal accusations and get back to the a constructive conversation about a technical problem. First, let's start a new thread since it has actually nothing to do with the 0.9.1 update.
Second, there are reasons that Roundcube comes with a custom session handler and those are still valid. We didn't do this just for fun, believe me!
The main reason are concurrent http request which alter session data. Image the following scenario:
A) [r]----------- big file being uploaded ------------[w] B) [r]----- fast upload ----[w]
In PHP, session data is read when the process starts and written when the process ends. With the scenario from above, the session changes of request B) would be lost when A) finishes.
I'm not a 100% sure I understand the example.
OK, here's another try. Imagine the following steps in chronological order:
Request A) read session data on start => $_SESSION['A'] == undefined => $_SESSION['B'] == undefined
Request B) read session data on start => $_SESSION['A'] == undefined => $_SESSION['B'] == undefined
Request B) $_SESSION['B'] = 'B'; writes session data when process ends => $_SESSION['A'] == undefined => $_SESSION['B'] == 'B'
Request A) $_SESSION['A'] = 'A'; writes session data when process ends => $_SESSION['A'] = 'A' => $_SESSION['B'] == undefined
Observed result: the entry 'B' in $_SESSION is lost after request A) wrote the session data because A) applied the changes to an outdated version of $_SESSION.
Can point out where this issue bubbles up in RoundCube?
It's a race condition that isn't simple to reproduce.
I just haven't heard of this issue either. But I'd like to understand.
It doesn't sound like databasing the sessions would be a solution either if one process wipes out another process' data.
I never said that using the database solves the issue. The main problem persists no matter what backend we would use.
Until now I don't know of any solution with default PHP session handlers that would be able to handle such a case properly. Another more performance related reason for you custom handler is the check if session data actually changed before issuing a (useless) update query.
Sure, we could store file upload information somewhere else than in session but the underlying problem of concurrent requests overlapping each others still persists and can happen in other cases, too.
As far as I understand, what happens in a request, stays in a request. I don't see how one request would override another.
It's not about two requests overriding each others memory but two requests reading and writing the same block of session data (either on disk or in the database).
The backend shouldn't matter. Databased sessions can have some advantages as they make scaling infrastructure easier but of course they could be considered overhead in a single-server setup.
Scalability was the main idea behind choosing the database to store session data. And since Roundcube requires a database connection anyway I don't see why this should be an overhead compared to file-based session storage.
I think the backend (file storage, database, memcache, redis) etc. should not have an impact unless we delete all data and re-save it every time. But anyway, let me know if you can explain this in more details.
As said, it's not mainly a backend issue here. What exactly do you want me to explain.
On Wed, May 22, 2013 at 9:00 PM, till klimpong@gmail.com wrote:
I agree that session handling could be nicer in RoundCube — or maybe not at all. ;-) If we wouldn't have customized session handling code we could let people configure PHP directly to use the filesystem, memcache or database. I think the least we could (or should) do is use an external library for session handling because that doesn't seem our domain (which is an email client).
Not sure if we could use something from Symfony2 or ZF2. They each provide wrappers — PHP 5.3, et all. Are people ready to move/upgrade yet?
We're open for suggestions to get rid of the custom session handlers but please keep the above scenario in mind. Switching to an external library might be a solution but don't they use custom save handlers, too?
They do — but RoundCube's session handling does something else which makes it incompatible with Suhosin.
And it would be very interesting to find out what we do wrong or different. The API for registering session save handlers is rather simple and I can't find anything we could change or improve when reading the docs again and again.
~Thomas
-
A.L.E.C -
Reindl Harald -
Thomas Bruederli -
till