sid not available in anonimous user

I just noticed that $user->sid is not available for
anonymous users.
It get chopped once $user get overwritten in sess_read even
if the
full session row get loaded. (5.X and 6.X).

include/session.inc

$user = db_fetch_object(db_query("SELECT u.*, s.* FROM
 u
INNER JOIN  s ON u.uid =
// $sid=$user->sid;

//...

  // We didn't find the client's record (session has
expired), or
they are an anonymous user.
else  {
    $session = isset($user->session) ? $user->session
: '';
    $user = drupal_anonymous_user($session);
//    $user->sid=$sid;
  }

since it makes more uniform to access sid through the code,
is there
a good reason to kill it?

If there isn't any good reason to kill it... could this
small change
get into core?

thx

-- 
Ivan Sergio Borgonovo
http://www.webthatworks.it

Quoting Ivan Sergio Borgonovo <mailwebthatworks.it>:

>
> If there isn't any good reason to kill it... could this
small change
> get into core?
>

Isn't it a problem that uid 0 has many sessions?

Earnie -- http://for-my-kids.com/
-- http://give-me-an-offer.
com/

On Wed, 16 Jan 2008 10:14:12 -0500
Earnie Boyd <earnieusers.sourceforge.net> wrote:

> Quoting Ivan Sergio Borgonovo <mailwebthatworks.it>:
> 
> >
> > If there isn't any good reason to kill it... could
this small
> > change get into core?
> >
> 
> Isn't it a problem that uid 0 has many sessions?

yes and no...
All anonymous users share one entry in the users table BUT
they don't
share the same row in the sessions table.

You can find what happens in include/sessions.inc sess_read

drupal load stuff from sessions table... but then discharge
it if the
user is anonymous reloading from drupal_anonymous_user.

I know that sid could be easily be loaded from $_SESSION but
a common
interface to registered/anon users would be nicer.

You call your function the same way passing $user->sid no
matter if
the user is authenticated or not.

I was wondering if there are any assumptions in the rest of
drupal
code about $user->sid for anonymous users...
Generally uid is checked... but maybe in some cleanup place
(eg.
logout, session expiration, whatever in the thousands lines
of code
of drupal) the assumption that $user->sid is not set is
made and I'd
like to have surprises.

If such assumption is not made... it would be nice if people
that
can commit on core made sid available even for anon users.

I already patched my drupal... but patching others code
without the
hope your patch get included upstream is always a
maintenance
nightmare on the long run.

If people find it a good idea to be able to get
$user->sid even for
not authenticated users... I don't mind about the
implementation ;)
If people think it is a bad idea, I'd like to know why.

thx

-- 
Ivan Sergio Borgonovo
http://www.webthatworks.it

> If people find it a good idea to be able to get
$user->sid even for
> not authenticated users... I don't mind about the
implementation ;)
> If people think it is a bad idea, I'd like to know
why.

i can't make out what your proposal is. please post a
patch.

On Wed, 16 Jan 2008 15:04:03 -0500
"Moshe Weitzman" <weitzmantejasa.com> wrote:

> > If people find it a good idea to be able to get
$user->sid even
> > for not authenticated users... I don't mind about
the
> > implementation ;) If people think it is a bad
idea, I'd like to
> > know why.

> i can't make out what your proposal is. please post a
patch.

Index: session.inc
============================================================
=======
--- session.inc (revisione 419)
+++ session.inc (copia locale)
 -48,6
+48,7 
   else  {
     $session = isset($user->session) ? $user->session
: '';
     $user = drupal_anonymous_user($session);
+    $user->sid=session_id();
   }

   return $user->session;

-- 
Ivan Sergio Borgonovo
http://www.webthatworks.it

i can't really get excited one way or another. if you want a
universal
way to get session id, call session_id(). your patch is a
very slight
improvement, i suppose.

On 1/16/08, Ivan Sergio Borgonovo <mailwebthatworks.it> wrote:
> On Wed, 16 Jan 2008 15:04:03 -0500
> "Moshe Weitzman" <weitzmantejasa.com> wrote:
>
> > > If people find it a good idea to be able to
get $user->sid even
> > > for not authenticated users... I don't mind
about the
> > > implementation ;) If people think it is a bad
idea, I'd like to
> > > know why.
>
> > i can't make out what your proposal is. please
post a patch.
>
> Index: session.inc
>
============================================================
=======
> --- session.inc (revisione 419)
> +++ session.inc (copia locale)
>  -48,6 +48,7 
>    else  {
>      $session = isset($user->session) ?
$user->session : '';
>      $user = drupal_anonymous_user($session);
> +    $user->sid=session_id();
>    }
>
>    return $user->session;
>
> --
> Ivan Sergio Borgonovo
> http://www.webthatworks.it

>
>

On Wed, 16 Jan 2008 23:10:36 -0500
"Moshe Weitzman" <weitzmantejasa.com> wrote:

> i can't really get excited one way or another. if you
want a
> universal way to get session id, call session_id().
your patch is a
> very slight improvement, i suppose.

I'm not excited as well... because it's wrong. Thank you for
pointing
it out in a gentle way (very slightly improvement
<g>).
It is not at all an improvement... it is error prone mixing
current
user sid with sid of an user object.
Previously the code was:

Index: includes/session.inc
============================================================
=======
--- includes/session.inc        (revisione 419)
+++ includes/session.inc        (copia locale)
 -30,6
+30,7 

   // Otherwise, if the session is still active, we have a
record of
the client's session in the database. $user =
db_fetch_object(db_query("SELECT u.*, s.* FROM 
u INNER JOIN
 s ON u.uid = s.uid WHERE s.sid = '%s'",
$key));
+  $sid=$user->sid;

   // We found the client's session record and they are an
authenticated user if ($user && $user->uid >
0) {
 -48,6
+49,7 
   else  {
     $session = isset($user->session) ? $user->session
: '';
     $user = drupal_anonymous_user($session);
+    $user->sid=$sid;
   }

   return $user->session;

The situation I'm trying to deal with are anonymous users
who own
temporary objects not suited to be placed in $session, eg.
e-commerce
carts.
I didn't use $key to fill $user->sid because I need a pk
in
.

Most of my functions accept a $user as argument so I can
manipulate
the current user or a user from the admin interface.
They see if the $user is authenticated, otherwise they
resort to the
sid to find the objects.

Does it make any more sense now?


Never pretend you can judge if your own code is elegant
near
midnight... especially if you went to sleep at 5:00am the
previous
day. Sorry.

-- 
Ivan Sergio Borgonovo
http://www.webthatworks.it

On Thu, 17 Jan 2008 12:01:57 +0100
Ivan Sergio Borgonovo <mailwebthatworks.it> wrote:

OK... I've found another thing that can't find a place in
the big
picture.
Even after login $user doesn't have $user->sid.
You can't find it in hook load and login.

While I can't find any better place to offer uniform access
to sid
for anonymous users other than the the below patch in core,
it is
very easy to add sid to $user object in a module... but well
more
than one module may need it as soon as the user is logged
in, and
since sid is getting into the $user object very soon later
and anyway
authenticated users have sid... why don't anticipate the
addition of
sid property into sess_regenerate?
While the below patch have a marginal cost in term of memory
(but
well even carriage return in code have) anticipating the
assignment
of sid property shouldn't have it.

Comments?

> Index: includes/session.inc
>
============================================================
=======
> --- includes/session.inc        (revisione 419)
> +++ includes/session.inc        (copia locale)
>  -30,6 +30,7 
> 
>    // Otherwise, if the session is still active, we
have a record of
> the client's session in the database. $user =
> db_fetch_object(db_query("SELECT u.*, s.* FROM
 u INNER JOIN
>  s ON u.uid = s.uid WHERE s.sid = '%s'",
$key));
> +  $sid=$user->sid;
> 
>    // We found the client's session record and they are
an
> authenticated user if ($user && $user->uid
> 0) {
>  -48,6 +49,7 
>    else  {
>      $session = isset($user->session) ?
$user->session : '';
>      $user = drupal_anonymous_user($session);
> +    $user->sid=$sid;
>    }
> 
>    return $user->session;
> 
> The situation I'm trying to deal with are anonymous
users who own
> temporary objects not suited to be placed in $session,
eg.
> e-commerce carts.
> I didn't use $key to fill $user->sid because I need
a pk in
> .

-- 
Ivan Sergio Borgonovo
http://www.webthatworks.it