Bugzilla – Bug 9830
server processed cookies fail if blank.
Last modified: 2009-07-31 10:31:16 UTC
in EN-based skins, SqueezeCenter-albumView cookie will not parse correctly once it is set to "". This happens while trying to toggle the artwork view. One possible fix is to change toggleGalleryView() in common.js to set the cookie to "0" instead. This may, however, mess up the Default skin, which looks for "" to mean text. Better fix would be to find out why Cookie:XS or CGI::Cookie can be made to parse correctly when a cookie is set to a blank string.
Kevin - do you have an idea whether this has shown up after the addition of Cookie::XS?
no idea. only located the problem finally just before I filed the report. too ill to do later nights than that, and it will have to wait for tonight to try anything more.
seems to be XS (at least on windows). If I force $hasCookieXS = 0; at line 75 of S::W::HTTP.pm the cookie is parsed correctly. Not a good record for the XS versions of modules so far :)
also, I notice that the reading of cookie when using XS forces the path to '/'. Could this be a problem when using the url-override? Cookies in the override case do show the skin name for the path. in fact, setting fishbone as the preferred skin makes it work again as well. I suspect that forced path is at least part of the problem. testing a possible patch.
doesn't seem to be a just the path issue. XS just fails to parse correctly with a blank cookie. Instead, it rolls right through to the next cookie, presenting the entire next cookie string as if it were the value of the cookie requested. If XS can't be easily fixed, it does seem that cookies are processed for every http request. perhaps we can limit to only certain file types?
Created attachment 4179 [details] only parse cookies for html files simple patch that stops cookie parsing for images and javascript. This avoids a cookie parse for every item within a page.
Michael: can you have a look at the patch from KDF. Please target this bug as well.
Kevin - this patch doesn't address your issue, but only reduces the number of requests which are parsed? This would allow to fix the main issue by not using XS?
yes, just reduces request count. it could be a boost in any case, but certainly helps given that I think XS is broken. I'm not into patching c modules and rebuilding in order to try some sort of fix there. If the request reduction helps enough, XS could be turned off for now and the bug forwarded to the authors.
thanks Andy for volunteering :-)
The XS cookie module changed namespaces and has a few fixes, can you test with http://search.cpan.org/~agent/CGI-Cookie-XS-0.16/ and see if it fixes this bug?
seems ok here (linux only since I cannot build binary modules elsewhere). installed .so in the server/CPAN/arch path and used the following patch on SC: Index: Slim/Web/HTTP.pm =================================================================== --- Slim/Web/HTTP.pm (revision 23802) +++ Slim/Web/HTTP.pm (working copy) @@ -71,7 +71,7 @@ $hasCookieXS = 0; eval { - require Cookie::XS; + require CGI::Cookie::XS; $hasCookieXS = 1; }; @@ -564,7 +564,7 @@ if ( my $cookie = $request->header('Cookie') ) { if ( hasCookieXS() ) { # Parsing cookies this way is about 8x faster than using CGI::Cookie directly - my $cookies = Cookie::XS->parse($cookie); + my $cookies = CGI::Cookie::XS->parse($cookie); $params->{'cookies'} = { map { $_ => bless {
OK, I'll see about updating to the newest version. Matt: Can you add CGI::Cookie::XS to the module build system in place of Cookie::XS? That'll take care of everything but the Windows and ReadyNAS builds.
crap...it looks like I was mistaken. The javascript was still cached, and was still setting the cookie to "0" instead of "". Clearing the cache, I still see the problem with the new version of Cookie::XS My initial patch posted here does reduce the items for which cookies are extracted, so perhaps there is a good speed gain there which would make it prudent to disable Cookie::XS for now without a huge speed penalty.
OK, why don't you apply your patch and we'll go back to non-XS for 7.3.
change 23859. I've left XS enabled, but it could be turned off by forcing hasCookieXS to return 1 until there is a working version or removing XS support entirely.
Disabled in change 23870.
This bug has been fixed in the 7.3.0 release version of SqueezeCenter! Please download the new version from http://www.slimdevices.com/su_downloads.html if you haven't already. If you are still experiencing this problem, feel free to reopen the bug with your new comments and we'll have another look.
Reduce number of active targets for SC