Title: Shrink/expand header persistence (SMF patch) Post by: PowerGlove on July 18, 2024, 06:14:49 AM This one was suggested (https://bitcointalk.org/index.php?topic=5503118.msg64330580#msg64330580) by dkbit98. (Thanks for kicking off the suggestions thread with an easy one; being able to quickly fix the first thing suggested makes me look badass, yo.) :D
The area at the very top of most pages looks like this: https://talkimg.com/images/2024/07/18/4QfTc.png That area can be "collapsed" (by clicking on the minus icon next to the date/time) to save some vertical space: https://talkimg.com/images/2024/07/18/4QyLP.png The problem is that the setting isn't being saved/persisted, so you have to keep collapsing it again and again as you navigate the site. What's interesting is that it is being persisted when you're not logged-in... That's weird, right? Why does it only remember to stay collapsed when you're not logged-in? It turns out that there are two ways in which this setting gets saved: When you're logged-in, it gets saved as a so-called "theme option" via a JavaScript function named smf_setThemeOption that depends on a support endpoint (/index.php?action=jsoption), and when you're not logged-in, it gets saved as a cookie. (At least, that's how it's supposed to work.) One of those two ways is still working (the cookie way), and the other is currently broken (the theme option way). I can only hazard a guess as to why smf_setThemeOption() no longer works, but I think what probably happened is that theymos disabled the support endpoint for it. Looking at the code for that endpoint, I can see why he might have done that (it's not a well-designed feature; it exposes too much attack surface relative to its usefulness). So, I can see two ways to fix the problem: Fix A Since the feature still works as intended by using cookies when you're a guest, this fix just makes it so that the feature always relies on cookies (whether you're logged-in or not). The advantage to fixing it this way is that the dodgy endpoint can stay disabled. The disadvantage is that the state is only persisted in a cookie, so if you sign in from a different device, or if you clear your cookies, then it'll "forget" (not a big deal, IMO). Code: --- baseline/Themes/default/index.template.php 2008-04-30 18:30:34.000000000 +0000 Fix B The other way to go would be to resurrect the disabled endpoint... The advantage to fixing it this way is that the state would be properly persisted and would effectively be "remembered" no matter what. There's also a very small traffic-wise advantage to not using cookies for this. The disadvantage is that the forum's attack surface would increase (with the below repair: negligibly, in my estimation, but still a strict increase). I've hardened the offending endpoint, and the result looks safe to me, but theymos should apply his mind carefully to the if-guard I've added to SetJavaScript() before deciding whether it's sensible to restore the endpoint or not (especially considering that there's an alternative fix). Code: --- baseline/Sources/Themes.php 2013-01-18 15:35:17.000000000 +0000 (Though I've described the problem only with respect to the collapsible section at the top of the page, there's another one, the so-called "info center" at the bottom of the main page, that these fixes also address.) Title: Re: Shrink/expand header persistence (SMF patch) Post by: NotATether on July 18, 2024, 07:43:18 AM I think making the forum use disabled endpoints is a bad idea, because theymos must have disabled it for security reasons as you know. So I would prefer the first patch. Even though I never collapse the header myself honestly - it's just confusing.
Although I am aware that newer versions of Simple Machines do handle this collapsing properly for logged in users. PS. Where are you pulling all these PHP files from - are they publicly accessible? And how do you test these things? Title: Re: Shrink/expand header persistence (SMF patch) Post by: joker_josue on July 18, 2024, 12:50:44 PM Fix A Since the feature still works as intended by using cookies when you're a guest, this fix just makes it so that the feature always relies on cookies (whether you're logged-in or not). The advantage to fixing it this way is that the dodgy endpoint can stay disabled. The disadvantage is that the state is only persisted in a cookie, so if you sign in from a different device, or if you clear your cookies, then it'll "forget" (not a big deal, IMO). By the way, I never used this feature much, but for those who use it a lot, the fact that the chosen option is not "memorized" can be annoying. I believe Fix A is the most appropriate. In addition to being simpler to apply, it will not affect the security of the forum. Furthermore, we are all used to there being times when a certain configuration stored in a cookie disappears, due to reset or use of another device. So this turns out not to be a big inconvenience. Because, as the person uses their browser, this should happen very occasionally. Congratulations PowerGlove on this patch. ;) Title: Re: Shrink/expand header persistence (SMF patch) Post by: Smartvirus on July 18, 2024, 04:57:48 PM Fix A Since the feature still works as intended by using cookies when you're a guest, this fix just makes it so that the feature always relies on cookies (whether you're logged-in or not). The advantage to fixing it this way is that the dodgy endpoint can stay disabled. The disadvantage is that the state is only persisted in a cookie, so if you sign in from a different device, or if you clear your cookies, then it'll "forget" (not a big deal, IMO). By the way, I never used this feature much, but for those who use it a lot, the fact that the chosen option is not "memorized" can be annoying. I believe Fix A is the most appropriate. Safe options are always going to be the best approach to have that which should the patches be considered, I guess “Patch A” would be the more appropriate. An option without risk of attack is better than having the memory comfort with a looming possibility of an attack. It’s an unpopular belief that many users would login their accounts on just any device. Mostly, users stick to their personal desktop/laptop computers or their mobile device. I don’t randomly pick up just any device to login my account, have reluctantly done that on few occasions but, it’s really that rare and in those moments, you don’t seek much comfort. Since the patch could be remembered on an already activated device, it’s cool. Title: Re: Shrink/expand header persistence (SMF patch) Post by: dkbit98 on July 18, 2024, 08:31:41 PM This one was suggested (https://bitcointalk.org/index.php?topic=5503118.msg64330580#msg64330580) by dkbit98. (Thanks for kicking off the suggestions thread with an easy one; being able to quickly fix the first thing suggested makes me look badass, yo.) :D Thanks PowerGlove, that was super-fast and quicker than I expected :)I am voting for Fix A because I think it is more simple to implement it, but we the forum members probably won't notice much difference if Fix B was selected by theymos. PS There are other elements in forum that have similar ''issue'', one of them is footer aka Info Center, but this is more useful than a header. Footer has Recent Posts and Forum Stats. |