Ticket #295 (closed bug: fixed)

Opened 3 years ago

Last modified 2 years ago

/tmp/fs_sessions no good with multiple users

Reported by: srl@… Owned by: omry
Priority: Normal Milestone:
Component: FireStats Version: 1.2
Severity: Normal Keywords:
Cc:

Description

Hello,

I have multiple users using firestats in wordpress. I made this patch so that they don't collide.

patch against 1.2.1-RC1

diff -Nur firestats.orig/php/session.php firestats/php/session.php
--- firestats.orig/php/session.php      2007-03-24 17:56:56.000000000 -0700
+++ firestats/php/session.php   2007-03-26 09:43:11.000000000 -0700
@@ -194,7 +194,7 @@
 
 function fs_initialize_session_dir()
 {
-       $temp_dir = sys_get_temp_dir();
+       $temp_dir = sys_get_temp_dir() . "/" . get_current_user();
        if($temp_dir === false)
        {
                // can't detect temp directory or php is running in safe mode.
@@ -205,11 +205,20 @@
 
 
        // make sure the dir ends with /
+       $help_url = "http://firestats.cc/wiki/ErrorInitializingSessionsDir";
+       $text = sprintf("<h3>Error initializing sessions directory, read <a href='$help_url'>this<a/> for help</h3><br/><span style='color:red'>%%s<span>");
        $last = substr($temp_dir, strlen( $temp_dir ) - 1 );
        if ($last != "/" && $last != '\\') $temp_dir .= "/";
+       if (!is_dir($temp_dir))
+       {
+               if (!mkdir($temp_dir, 0700))
+               {
+                       die(sprintf($text,"Failed to create '<b>$temp_dir</b>'"));
+               }
+       }
        $temp_dir .= SESSIONS_DIR;
-       $help_url = "http://firestats.cc/wiki/ErrorInitializingSessionsDir";
-       $text = sprintf("<h3>Error initializing sessions directory, read <a href='$help_url'>this<a/> for help</h3><br/><span style='color:red'>%%s<span>");
+       $last = substr($temp_dir, strlen( $temp_dir ) - 1 );
+       if ($last != "/" && $last != '\\') $temp_dir .= "/";
        if (!is_dir($temp_dir))
        {
                if (!mkdir($temp_dir, 0700))
@@ -217,6 +226,8 @@
                        die(sprintf($text,"Failed to create '<b>$temp_dir</b>'"));
                }
        }
+
+
        if (!fs_is_writable($temp_dir))
        {
                die(sprintf($text,"Directory ,'<b>$temp_dir</b>' must be writable"));

Attachments

Change History

Changed 3 years ago by omry

since the session file name are computed using random number and a timestamp, the chance of a collision is infinitesimally small.

besides, FireStats core can't depend on any WordPress specific feature. (like get_current_user()).

Changed 3 years ago by srl@…

yes, but you are running mkdir 0700 - so the other users can't read or write to /tmp/fs_sessions

get_current_user() is PHP, i think.. http://php.net/get_current_user but actually it looks like it should be getmyuid() in case the script's owner is different.. maybe something like this:

$tmpdir . "fs_sessions" . getmyuid()

that should make /tmp/fs_sessions.501 etc for UID 501

An alternative route could be making the directory a+rwxt..

Changed 3 years ago by omry

now I understand: you mean you have multiple system users accessing the same instance of FireStats. in that case it makes sense.

I will evaluate your patch when I return from my vacation in a few weeks (with the suggestion of getmyuid())

Changed 3 years ago by omry

  • status changed from new to closed
  • resolution set to fixed

added support for multiple system users, creating fs_sessions/uid directories. this will be released with the 1.2.2, I will apprecisate it if you make sure it works for you.

Changed 3 years ago by omry

I am using postix_getuid to create the user specific directory. if its not available I am skipping it (obviously) and getting the usual /tmp/fs_sessions.

can you verify this solution works for you?

http://files.firestats.cc/firestats-1.2.2-RC2.zip

Add/Change #295 (/tmp/fs_sessions no good with multiple users)

Author



Change Properties
<Author field>
Action
as closed
Next status will be 'reopened'
 
Note: See TracTickets for help on using tickets.