Code Monkey home page Code Monkey logo

Comments (24)

joshuaadickerson avatar joshuaadickerson commented on May 24, 2024

I don't think I've ever heard of someone saying that they have had conflicts. What we should do here is check if another session handler is enabled and tell them that they will probably get better performance from using that.

from smf.

butch2k avatar butch2k commented on May 24, 2024

Frankly i never understood why SMF stores sessions in DB, performance wise it's far from being optimal (especially with myisam tables), default PHP "files" handler is much faster.

I think i tracked down the issue to igbinary being set as the session serializer, indeed content of the smf_session table is corrupted, here is the kind of stuff i get:
�������
session_value� 6d355afb92a7adfd5db6fc5fc303308b��session_var��d833942��id_msg_last_visit��1��mc����time
P

Since the sessionWrite handler of SMF assume the data is a string, stuff saved gets corrupted since session data is binary.

from smf.

Arantor avatar Arantor commented on May 24, 2024

It's not performance, it's reliability and security related. If you use the file handler on a shared host, aside from the fact that anyone on the shared host will be able to access them, the files are not likely to hang around very long, or worse, collide with another app's session on the server.

IMNSHO, it should stay as is for default, and anything else on a case by case basis.

from smf.

butch2k avatar butch2k commented on May 24, 2024

Knowledgeable PHP admins would configure the session save path by vhost
rather than using a common one. I do not criticize the fact that one could
chose to use db sessions, it's the fact that it's made default that i'm
wondering about.
Indeed using MyIsam table for session data could be a real killer for
shared hosts ! Keep in mind that myisam use a table locking mechanism on
updates and inserts. With a sufficiently large number of users or a slow
host it could cause a large slowdown compared to file based sessions.

On Wed, Jan 16, 2013 at 2:02 PM, Arantor [email protected] wrote:

It's not performance, it's reliability and security related. If you use
the file handler on a shared host, aside from the fact that anyone on the
shared host will be able to access them, the files are not likely to hang
around very long, or worse, collide with another app's session on the
server.

IMNSHO, it should stay as is for default, and anything else on a case by
case basis.


Reply to this email directly or view it on GitHubhttps://github.com//issues/277#issuecomment-12317392.

from smf.

emanuele45 avatar emanuele45 commented on May 24, 2024

Do you know more "knowledgeable PHP admins" or more admins that don't even know what php is?

from smf.

butch2k avatar butch2k commented on May 24, 2024

yeah i know... hopefully some panel distros tackle this issue.

On Wed, Jan 16, 2013 at 2:32 PM, emanuele45 [email protected]:

Do you know more "Knowledgeable PHP admins" or more admins that don't even
know what php is?


Reply to this email directly or view it on GitHubhttps://github.com//issues/277#issuecomment-12318466.

from smf.

Arantor avatar Arantor commented on May 24, 2024

The point that you are trying so hard to dance around is that 'faster is not necessarily better'.

Seriously, this should be left alone. The default is sane and easily recommended for shared hosts. The people who need to change it are the people who need to wring every ounce of performance out of it, and are running on a setup where they can safely make the switch.

Of course, you could make it even faster than files by making it a memory table instead of a MyISAM one. In fact, even InnoDB with the right configuration should be faster than a MyISAM one.

Yes, using MyISAM could be a killer - just like anything else. But I'd rather people had potential (rather than guaranteed) performance issues rather than being locked out or having their account compromised.

Security trumps performance, IMO.

from smf.

butch2k avatar butch2k commented on May 24, 2024

It's not an "the faster the better" issue, in my current use case (using
igbinary serializer) you just can't login into the admin panel if you rely
on SMF DB sessions with this serializer active, and it's probably the same
with the more standard php_binary serializer. So i was effectively locked
out... i had to manually change de value in the settings table.

On Wed, Jan 16, 2013 at 2:45 PM, Arantor [email protected] wrote:

The point that you are trying so hard to dance around is that 'faster is
not necessarily better'.

Seriously, this should be left alone. The default is sane and easily
recommended for shared hosts. The people who need to change it are the
people who need to wring every ounce of performance out of it, and are
running on a setup where they can safely make the switch.

Of course, you could make it even faster than files by making it a memory
table instead of a MyISAM one. In fact, even InnoDB with the right
configuration should be faster than a MyISAM one.

Yes, using MyISAM could be a killer - just like anything else. But I'd
rather people had potential (rather than guaranteed) performance issues
rather than being locked out or having their account compromised.

Security trumps performance, IMO.


Reply to this email directly or view it on GitHubhttps://github.com//issues/277#issuecomment-12318911.

from smf.

Arantor avatar Arantor commented on May 24, 2024

So, let me get this straight, you're actively recommending changing the default in SMF for all users, which would put most users in a worse position than they are in now, just because it's broken for you?

Seems to me you could have solved it by making it a blob column in the DB, but again that's specific to your use case - if it were a mass problem, it would have been all over sm.org by now.

from smf.

butch2k avatar butch2k commented on May 24, 2024

It's just not me, it's a reproductable use case resulting in a broken
install with no explicit reasons given.
Of course i could have solved the issue by using a blob, it's not the issue
i'm underlining (in fact i'm not using db nor filesystem sessions). It's
not a real issue for me as i know how to correct it, but i'm not certain
anyone could correct it in case this happens.

I believe there should be at least some warning during install about binary
serializers not being compatible with db sessions, or set the serializer to
standard php within SMF when using db sessions so as to prevent such issues.

On Wed, Jan 16, 2013 at 3:40 PM, Arantor [email protected] wrote:

So, let me get this straight, you're actively recommending changing the
default in SMF for all users, which would put most users in a worse
position than they are in now, just because it's broken for you?

Seems to me you could have solved it by making it a blob column in the DB,
but again that's specific to your use case - if it were a mass problem, it
would have been all over sm.org by now.


Reply to this email directly or view it on GitHubhttps://github.com//issues/277#issuecomment-12320983.

from smf.

Arantor avatar Arantor commented on May 24, 2024

Show me ONE other case on sm.org where this has happened. Any one, don't mind which.

There would be many other problems attached, too, you'd also need to consider making the settings table a blob.

So, again, we're back to the fact that you're wanting a core change in SMF because you're using a non-standard extension that hasn't come up any other time.

That said, I would grudgingly admit that forcing the serializer back to PHP for DB sessions should be done, but mostly out of a sense of preventing other things screwing it up; pretty much anyone installing igbinary would be wary of the consequences, not because it's actually necessary - because it isn't. You're not using the standard handling, therefore what you're describing is still a fringe case.

(This is why I'm not an SMF developer, I don't have to actually worry about denying this request :P)

from smf.

butch2k avatar butch2k commented on May 24, 2024

Olivier Lefebvre
Sent with Sparrow (http://www.sparrowmailapp.com/?sig)

Le mercredi 16 janvier 2013 à 19:08, Arantor a écrit :

Show me ONE other case on sm.org where this has happened. Any one, don't mind which.
There would be many other problems attached, too, you'd also need to consider making the settings table a blob.
So, again, we're back to the fact that you're wanting a core change in SMF because you're using a non-standard extension that hasn't come up any other time.
That said, I would grudgingly admit that forcing the serializer back to PHP for DB sessions should be done, but mostly out of a sense of preventing other things screwing it up; pretty much anyone installing igbinary would be wary of the consequences, not because it's actually necessary - because it isn't. You're not using the standard handling, therefore what you're describing is still a fringe case.
(This is why I'm not an SMF developer, I don't have to actually worry about denying this request :P)


Reply to this email directly or view it on GitHub (#277 (comment)).

from smf.

butch2k avatar butch2k commented on May 24, 2024

I'll look for one ;-)
I'm remember some issue with accessing admin center or being unable to logout.

My main complain is that a working system relying on two components: redis & igbinary is overridden in such a way that the system is not working anymore.

I agree it's an edge case but I think using the correct serializer would be more consistent.

Olivier Lefebvre
Sent with Sparrow (http://www.sparrowmailapp.com/?sig)

Le mercredi 16 janvier 2013 à 19:08, Arantor a écrit :

Show me ONE other case on sm.org where this has happened. Any one, don't mind which.
There would be many other problems attached, too, you'd also need to consider making the settings table a blob.
So, again, we're back to the fact that you're wanting a core change in SMF because you're using a non-standard extension that hasn't come up any other time.
That said, I would grudgingly admit that forcing the serializer back to PHP for DB sessions should be done, but mostly out of a sense of preventing other things screwing it up; pretty much anyone installing igbinary would be wary of the consequences, not because it's actually necessary - because it isn't. You're not using the standard handling, therefore what you're describing is still a fringe case.
(This is why I'm not an SMF developer, I don't have to actually worry about denying this request :P)


Reply to this email directly or view it on GitHub (#277 (comment)).

from smf.

Arantor avatar Arantor commented on May 24, 2024

There we go ;) I'm not disagreeing that there's an issue, I'm just not convinced it needed to be fixed in the core. Now, on the other hand, if using redis for sessions was made a core feature, or even memcached, I'd have no hesitation in suggesting adding @ini_set('session.serialize_handler', 'php'); before the call to session_set_save_handler in Load.php...

Thing is, issues with accessing admin or being unable to log out could be all kinds of other session trouble, including using file sessions and having them hijacked.

from smf.

Arantor avatar Arantor commented on May 24, 2024

Actually, now that I think about it, it's even more complicated than has been suggested.

It's not just sessions that rely on serialisation behaviour - admin/moderation/profile edits log, package manager/DB edits log, and who's online do too.

On top of that, who's online, when turned on and set up to check who is viewing a board or topic will not merely be broken in terms of the DB, it just won't work, because it specifically relies on querying the online log table, looking for ?board= and ?topic= type entries in the log, which can't work in the event of binary serialisation.

This means there are two routes - rearchitecturing everything to support blob columns (and dealing with schema changes to the who's online log), or setting php serialisation up front without any qualms - which would mostly negate any minor performance benefits you're getting out of it.

from smf.

butch2k avatar butch2k commented on May 24, 2024

you mean the cache_put_data/cache_get_data ?

They use the standard php serialization behavior through the serialize
function, to override this behavior you need to call igbinary_serialize so
there is no trouble there. It's really just the DB sessions which are at
fault because of session.serialize_handler this parameter sets the
serializer for sessions only.

On Wed, Jan 16, 2013 at 9:28 PM, Arantor [email protected] wrote:

serialisation

from smf.

Arantor avatar Arantor commented on May 24, 2024

No, I don't mean the cache. I mean all these tables that have serialised data directly in them, but if you have to use a separate function (the docs don't seem very clear to me) then it won't be a problem.

from smf.

butch2k avatar butch2k commented on May 24, 2024

Ok, i understand.
There is no issue with calls to serialize/unserialize, it's just the
session serialization which is at fault here.

On Wed, Jan 16, 2013 at 9:52 PM, Arantor [email protected] wrote:

No, I don't mean the cache. I mean all these tables that have serialised
data directly in them, but if you have to use a separate function (the docs
don't seem very clear to me) then it won't be a problem.


Reply to this email directly or view it on GitHubhttps://github.com//issues/277#issuecomment-12339366.

from smf.

emanuele45 avatar emanuele45 commented on May 24, 2024

I think i tracked down the issue to igbinary being set as the session serializer

Was wondering is there a way to detect if igbinary is used (not only present, but actually used) or not?
If so we could add a warning somewhere...

from smf.

butch2k avatar butch2k commented on May 24, 2024

It should be as simple as checking if php session serializer value is set
to igbinary or php_binary no ?

On Mon, Jan 28, 2013 at 11:39 PM, emanuele45 [email protected]:

I think i tracked down the issue to igbinary being set as the session
serializer

Was wondering is there a way to detect if igbinary is used (not only
present, but actually used) or not?
If so we could add a warning somewhere...


Reply to this email directly or view it on GitHubhttps://github.com//issues/277#issuecomment-12809646.

from smf.

emanuele45 avatar emanuele45 commented on May 24, 2024

Can you tell me what:

echo ini_get('session.serialize_handler');

returns for you? (just to be sure and not to send around useless warnings)

Reading the php documentation is should return php or php_binary for the "normal" handler.
How many other handlers are available?
Is igbinary the only one causing issues or there are others as well?
Just asking since I have not much experience with sessions.

from smf.

butch2k avatar butch2k commented on May 24, 2024

it returns

igbinary

from smf.

butch2k avatar butch2k commented on May 24, 2024

Some background on the serializers:
http://stackoverflow.com/questions/6722874/what-is-the-php-binary-serialization-handler?lq=1

besides php, php_binary, and igbinary serializers there is wddx which is xml based (i do not know of others)

i haven't tested php_binary but it might have the same behavior as igbinary.

from smf.

Arantor avatar Arantor commented on May 24, 2024

After reviewing this, the simplest solution really is just to ini_set the serializer before initialising the SMF DB handling stuff. That way it won't materially affect anyone using the DB sessions, and anyone not using that can use whatever serializer they wanted to use in the first place and let whatever backend deal with it.

from smf.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.