Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Issues with sessions #32

Open
catwell opened this Issue Sep 2, 2013 · 13 comments

Comments

Projects
None yet
4 participants
Contributor

catwell commented Sep 2, 2013

Hello,

I used Lapis over the weekend, congats on the progress!

I think I found two little bugs with sessions, both in the write_session function.

__index can be a function

I did not have enough time to find out why that happens but __index in the session metatable can be a function, so when write_session tries to iterate it assuming that it is a table it breaks.

Removing the last key-value pair fails

When you write the session you start by checking that it is not empty. The problem is that maybe it is empty now but it was not empty earlier. If the session is empty you should make sure it is removed on the client side.

This is easy to find out if you make a simple project with two routes, one to login which sets the current user like in the docs:

@session.current_user = "leaf"

and the other one to logout which removes it:

@session.current_user = nil

This will not work if there is nothing else in the session (logout is impossible). A possible workaround in current state is replacing the logout code with one of those:

@session.current_user = false

or

@session._dummy = true
@session.current_user = nil
Owner

leafo commented Sep 3, 2013

I'm aware of the session removal issue. I've been using the false assignment workaround in my sites, but I think it's time to make it a little bit more proper and detect nil assignments.

As for __index being a function, I've never seen that happen before, but I see no issue with adding a check there.

Contributor

catwell commented Sep 3, 2013

Thank you. Regarding __index being a function I will try to make a simple test case that exhibits the problem this weekend.

Contributor

catwell commented Sep 3, 2013

Actually the 2nd workaround does not work, it did only because I had commented out code (using false instead of nil does). And I couldn't reproduce __index being a function so disregard that for now.

iandyh commented Sep 6, 2013

@leafo I am currently experiencing the __index being a function issue. Completely have no idea why this would happen. I have dig around but found no solutions.

So I use cookies instead now.

Contributor

catwell commented Sep 6, 2013

I have had a look and here's where I think the issue with __index being a function comes from. Well, that's if I have understood all this meta-magic correctly :)

OK, so the session is an auto_table. An auto_table is a table with a __index metamethod by default (so "__index is a function").

The reason this bug does not happen all the time is that actually the __index metamethod is a closure, and when it is called it replaces itself with the result of the enclosed fn (a table). So if you access a field in the session before trying to iterate __index it works, but not if you don't.

Owner

leafo commented Sep 6, 2013

Ah good catch, thanks for looking into it.

@leafo leafo added a commit that referenced this issue Sep 7, 2013

@leafo leafo improve last commit #32 c018818

@leafo leafo added a commit that referenced this issue Sep 7, 2013

@leafo leafo add test for auto_table session #32 edb1ca1

kotedo commented Sep 10, 2013

Question: Has the session issue been fixed? I am using the latest github checked out version and I am still having the aforementioned session issues.

Thank you!

Owner

leafo commented Sep 10, 2013

Still haven't changed the delete issue. The __index being a function has been fixed.

kotedo commented Sep 10, 2013

Right, that's what I noticed. It renders sessions almost unusable ... and I was hoping to use Lapis for an important site that requires sessions.

Owner

leafo commented Sep 10, 2013

I wouldn't say they're unusable. Set to false if you want to delete. I'll try to get patch out asap.

Owner

leafo commented Oct 1, 2013

So I've taken care of the assigning nil issue, but I'm not satisfied with the overall result. You will still run into trouble if you have a table serialized into the session and you modify it, the code won't be able to detect the change in the child object. I'm wondering if I should just change the interface completely, have some method that marks the session as dirty and forces it to be written, or write something that wraps the objects that come from the session so I can tell if any nested object is changed.

Contributor

catwell commented Oct 1, 2013

That last solution looks right. What can be stored in sessions exactly? Userdata, functions? If neither can then just adding metatables to tables stored in sessions so that they propagate the "modified" state upwards sounds good.

Owner

leafo commented Oct 1, 2013

Anything that can be serialized by lua cjson. I should mention that somewhere.

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