-
Notifications
You must be signed in to change notification settings - Fork 253
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[2.1] & [3.0]: Consider disabling URL-based sessions, getting rid of PHPSESSID from URLs #8383
Comments
This rewrites all instances of $scripturl except the canonical. It’s find a link, unless it’s preceded by the canonical rel, gotta watch the backwards assertions. It targets every link that matches the base form of $scripturl except the canonical because that you definitely don’t want the session ID injected into, before going on to apply the queryless URLs changes immediately after (which is why you get the PHPSESSID where you do in those URLs), and note the forward assertion on not being followed by a ? so you don’t inject the PHPSESSID twice. You can cut the lines in there that reference SID or session_id() and the entire behaviour will just stop, though that would suggest renaming the buffer rewrite function after since it wouldn’t be rewriting the session into links any more. |
Yep, that's correct. Initial description updated. |
I honestly can't think of a reason to still do it. I am guessing the only reason was in the old days browsers may not support cookies so this supplemented support for them by ensuring we passed the session along. This does mean that its possible for bots to generate new sessions on every page click, wasting resources, but I don't think that is as much of a problem nowadays. |
Yup, that was the rationale - back in 2003 cookie support in bots (in particular) was flaky as anything so you kind of needed it to neutralise the worst offenders causing you drama. |
There are multiple places 2.1 stuffs PHPSESSID in the URL... (3.0 is a little tidier, & uses a common function.) So removing PHPSESSID requires making the PHP session setting change, & removing all SMF PHPSESSID logic. In 2.1, this will include at least redirectexit & ob_sessrewrite. One downside I've seen testing this configuration so far is that sessions proliferate pretty bad, 1-3 new sessions per click. We've shut off all the paths for session reuse. Same behavior for bots. Given they aren't being reused, I think we should consider suppressing session creation where cookies don't exist. |
To your last point as I said on SMF: if you don’t create a session until there’s a cookie, how does anyone get a cookie to initiate the session? The auth cookie is only viable for authenticated users, and you need a working session to authenticate owing to CSRF. Session proliferation sounds daisy chained requests - the immediate assumption is that a first request opens a session, then the cron/scheduled tasks calls might trigger another session because they’re called sufficiently early that the first session hasn’t necessarily closed yet? Seems like a logical case - and those shouldn’t ever be depending on a session anyway. |
@live627 pointed out online that use_cookies only is deprecated as of PHP 8.4: https://www.php.net/manual/en/migration84.deprecated.php#migration84.deprecated.session They are removing those settings, as part of eliminating PHP putting PHPSESSID in the URL. Deprecation notices start in 8.4, & the functions are completely removed in 9.0. At that point, the only (straightforward) way to pass the session will be via cookie. So... When we remove the SMF PHPSESSID logic, we should also prepare for 8.4 by removing references to use_only_cookies & use_trans_sid. In PHP 8.3, the SMF PHPSESSID logic is effectively shunted with use_only_cookies set to true, because with that setting, SID is always an empty string. (Which also happens whenever a cookie is present.) This effectively removes PHPSESSID from URLs as well. I also think that once we remove PHPSESSID from the URL, AND, we encounter a situation where cookies are disabled, we should not bother writing the session. Why waste the resources? There is no point in keeping a session around that cannot be reused. Writing sessions can be expensive... At points over the last few months, I was getting nearly ~200K bot hits a day. In the MySQL reports provided by my host, writing sessions was at times my highest cumulative cost. Individually tiny, but they add up with volume. Not worth keeping around if they are not used. |
Basic Information
See discussion here:
https://www.simplemachines.org/community/index.php?msg=4184207
It's pretty easy to make the change in Sessions.php. And yes, PHPSESSID does go away with that one line change. (Set session.use_only_cookies to true.)
I haven't experimented a lot with how far you get with cookies disabled in SMF today. I'm pretty sure you can't login anyway, i.e., you might not be losing anything at all by disabling URL-based sessions. And bots already blow up the most-online-today stats. Having PHPSESSID is the exception, not the rule these days.
While in there, we need to look closely at this logic, which rewrites the urls in head:
SMF/Sources/QueryString.php
Line 424 in 5a0150e
This appears to be an attempt to minimize creation of multiple sessions.
Related discussion:
#8367
Steps to reproduce
Expected result
No response
Actual result
No response
Version/Git revision
3.0 Alpha 2 - current GH & 2.1
Database Engine
All
Database Version
8.4
PHP Version
8.3.8
Logs
No response
Additional Information
No response
The text was updated successfully, but these errors were encountered: