Open tyler-sommer opened 9 years ago
Hi Tyler,
Thanks very much for your work here, I'm going to have a more thorough read of your suggestions when I get back from holiday on Tuesday. This is just a quick note as I didn't want you to think I was ignoring the issue!
Sorry for closing and reopening - I'm on a mobile with fat fingers!
Andrew
Hi Tyler,
Very glad to hear that PHPFastCGI is speeding up your app! Is that 400ms or 40ms? If it's 400ms, your app must be doing quite a lot!
I've just done some research on sessions! I've never really checked out the internals of PHP session handling or used the Symfony session classes before.
A couple of quick points before I get into it. My understanding always was that if you could obtain the session identifier then that could be used to hijack the session. On that note, the method you use to generate the session identifer in the example given isn't cryptographically secure. The output of both mt_rand() and uniqid() is predictable and shouldn't be used for these purposes. I know that this is example code, but I thought that I should mention it anyway!
Moving on, my current thinking is that whilst it does make sense for the core FastCGIDaemon package to provide the ability for integrations to work with sessions, the actual logic for their implementation is outside the scope of the package. To include this would require the package to be coupled to some form of storage layer - and I don't think this would be a good idea. Also, the $_SESSION suplerglobal isn't something that PHPFastCGI applications should be using. I would recommend that applications using the FastCGIDaemon without a framework adapter should employ some form of session middleware/service that is capable of handling multiple request cycles.
All that said, where a framework does provide a wrapper for handling sessions, the PHPFastCGI adapter should maintain the functionality of this wrapper and not break it (as it currently does with Symfony). A well designed framework should provide some pathway to doing this!
With the Speedfony Bundle for Symfony - I think the best approach would be to override the session service by setting the "session.class" parameter. I'm going to have a play with this tomorrow to test if the session storage interface (injected into the session service) is flexible enough to allow for multiple request cycles. If not, we're going to have a problem (and a few suggestions for Symfony 3!). From what I can tell, the bundle may also need to register an extra response listener to set the session cookie, as currently the Symfony framework relies on that being handled by PHP itself.
I've got a bit of refactoring to do on the core FastCGIDaemon package over the next few days. That gives us some time to keep discussing this - and then I can look towards implementing it.
Would be interested to know your thoughts!
Andrew
With the Speedfony Bundle for Symfony - I think the best approach would be to override the session service by setting the "session.class"
@AndrewCarterUK Don't rely on .class
parameters please. We're not introducing them to new services, and were considering removing them for existing ones. Either override the service or decorate it.
I ran into some issues with handling multiple simultaneous sessions.
@tyler-sommer What issues exactly? I'm not sure what are we trying to solve here. Would it be possible to reproduce your problems on a vanilla Symfony standard edition? We need to rule out the possibility the problem lies in your project.
Thanks @jakzal.
The issue is that the default configuration for Symfony lets PHP append the session cookie when the application sends out the response. Out of the box, Symfony pretty much uses the default PHP session_*() api. This doesn't work with PHPFastCGI, because the response doesn't go through PHP and thus the PHPSESSID cookie is never set.
Is that 400ms or 40ms? If it's 400ms, your app must be doing quite a lot!
I'm not sure where I got that number from. A little more testing shows a pretty consistent ~100ms improvement, however.
On that note, the method you use to generate the session identifer in the example given isn't cryptographically secure. The output of both mt_rand() and uniqid() is predictable and shouldn't be used for these purposes. I know that this is example code, but I thought that I should mention it anyway!
Thanks for pointing this out! I looked into it a bit, and it seems the PHP implementation for session id generation does a combination of things. Here's a nice summary on Stack Overflow.
I think it would be ideal if we could continue to depend on PHP for id generation, though it seems like the very best solution would be to completely decouple from the language-provided mechanisms. However, it also seems like session management is not completely trivial-- as you saw me demonstrate.
Moving on, my current thinking is that whilst it does make sense for the core FastCGIDaemon package to provide the ability for integrations to work with sessions, the actual logic for their implementation is outside the scope of the package. To include this would require the package to be coupled to some form of storage layer - and I don't think this would be a good idea. Also, the $_SESSION suplerglobal isn't something that PHPFastCGI applications should be using. I would recommend that applications using the FastCGIDaemon without a framework adapter should employ some form of session middleware/service that is capable of handling multiple request cycles.
All that said, where a framework does provide a wrapper for handling sessions, the PHPFastCGI adapter should maintain the functionality of this wrapper and not break it (as it currently does with Symfony). A well designed framework should provide some pathway to doing this!
This makes sense. I think that probably the best way to provide that pathway is through documentation, though I also wonder if there might be a place for a separate library specifically suited to multi-request session management. That might help remove the guesswork and stop people from making simple mistakes in their own implementations.
Maybe try to invest into https://github.com/Ocramius/PSR7Session ?
I've successfully gotten a fairly large Symfony 2 application working under FastCGIDaemon. Initial testing shows up to 400ms saved per request!
However, I ran into some issues with handling multiple simultaneous sessions. I gutted the Symfony 2 handling (other than HttpFoundation Request/Response) to try to get an understanding of how PHP is handling things. Here's what I've got so far:
This works, though obviously ugly. Key points:
session_get_cookie_params()
so that the ini settings could be used.Thoughts:
SessionHandlerInterface
will invariably be a better solution. This also gives you a clear place to implement the regenerating of a session_id (by setting acreate_sid
callback [yes, that means the oldsession_set_save_handler
prototype must be used]).session.use_cookie
might not be necessaryI'm concerned that I'm missing something as far as maintaining session security (preventing hijacking or fixation). Since this code blindly checks for a
PHPSESSID
cookie, it would be a trivial affair to hijack someone else's session, if you could guess the ID. However, I don't think this logic differs from the built-in default PHP session_id validation logic, so maybe it's a moot point.