When making a new search or navigating the search result pages, catalogue/search.pl->browser.js generates a search cursor cookie for every page change or search. When 30+ search/page change -actions are done, browser has collected 30+ search cursor cookies. This causes the HTTP-header to bloat and get rejected by apache2. User needs to reset cookies to get rid of this issue and be able to continue using Koha. ------------ - SOLUTION - ------------ Name searchCookies (search cursor cookies) as "scs_<timestamp>" and remove the 3 oldest cookies if more than 15 searchCookies are present.
Sponsored by the Joensuu Regional Library
Created attachment 23389 [details] [review] Bug 11369 - Updating the jquery.cookie.js-plugin The current jquery.cookie-plugin crashes when trying to fetch all cookies using $.cookie(); Downloaded the newest plugin version and minified it. Now works as intended. Encountered an issue with the plugin now returning null when no cookies are found, and applied a fix in browser.js. ------------- - Test plan - ------------- Plugin is used in browser.js and batchMod.js so testing both --- browser.js --- In catalogue/search.pl look for any record. You should get a search cookie stored in your browsers cache with the following format: 4Xea40qp : {"offset":1,"query":"idx=kw&q=read","limit":"","sort":"","pagelen":20,"results":[1430,2253,2255,2256,2257,2258,2307,2326,2344,2595,2961,3588,3974,3995,4026,4405,4410,4412,4414,4435]} Select any record for detailed view. You should get a navigation panel on your left part of screen. You should be able to move to the previous and next record and return to results. --- batchMod.js --- Select a handful of barcodes to batch modify, then continue. The cookie affected stores the columns you want displayed. Try showing/hiding columns. When you make another batch modification attempt, your column selection persists via the showColumns-cookie.
Created attachment 23390 [details] [review] Bug 11369 - Too many search cursor cookies overflow HTTP-header size, when making multiple searches in the staff client. To replicate this issue, make multiple searches in catalogue/search.pl. 50+ Should be enough to cause the HTTP-request header to overgrow. One can verify this issue by observing the searchCookie growth in browser's stored cookies. ------------- - TEST PLAN - ------------- Keep making searches. One should never have more than 10 searchCookies. Browser might display only 9, because for some reason the newest js-generated cookie is not included in Firefox's cookies listing. ------------ - DRAWBACK - ------------ Removing these cookies disables the search cursor for traversing search results (next/previous) for the removed cookie. This maybe be problematic in some cases, (for ex when multiple search tabs need to be open and they need to be traversed) One easy solution is to grow the amount of stored searchCookies from 10 to 20, but 10 is chosen so there will be plenty of room for other cookies as well.
Sponsored by Joensuu Regional Library
I did not test this patch but the issue makes me think to bug 10952. Julian fixed the issue storing the values in session instead of cookies.
Created attachment 25662 [details] [review] Bug 11369 - Updating the jquery.cookie.js-plugin The current jquery.cookie-plugin crashes when trying to fetch all cookies using $.cookie(); Downloaded the newest plugin version and minified it. Now works as intended. Encountered an issue with the plugin now returning null when no cookies are found, and applied a fix in browser.js. ------------- - Test plan - ------------- Plugin is used in browser.js and batchMod.js so testing both Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Created attachment 25663 [details] [review] Bug 11369 - Too many search cursor cookies overflow HTTP-header size, when making multiple searches in the staff client. To replicate this issue, make multiple searches in catalogue/search.pl. 50+ Should be enough to cause the HTTP-request header to overgrow. One can verify this issue by observing the searchCookie growth in browser's stored cookies. ------------- - TEST PLAN - ------------- Keep making searches. One should never have more than 10 searchCookies. Browser might display only 9, because for some reason the newest js-generated cookie is not included in Firefox's cookies listing. ------------ - DRAWBACK - ------------ Removing these cookies disables the search cursor for traversing search results (next/previous) for the removed cookie. This maybe be problematic in some cases, (for ex when multiple search tabs need to be open and they need to be traversed) One easy solution is to grow the amount of stored searchCookies from 10 to 20, but 10 is chosen so there will be plenty of room for other cookies as well. Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
I've signed with a few trivial changes : - I've set : scsCookieKeys.length >= 10 instead of > to maintain 10 cookies max. - I've removed '<<' from '//EO Bug_11369' because it is better not using several '<' like in patch conflicts (<<<<<<). @Jonathan indeed, the could use session but it seems that it tries to use maximum javascript only. For me best of all would be using the HTML5 SessionStorage feature.
Created attachment 25667 [details] [review] Bug 11369 - Correct usage of me.searchid in browser.js In browser.js, at creation of browser, the searchid is transmited to JS object into me.searchid. To be consistant, me.searchid should always be use, never searchid alone. In browseRecords function, setting searchid as parameter is useless because it is defined in me.searchid. Test with test plan of Bug 10404
Created attachment 25669 [details] [review] Bug 11369 - General searchid in browser.js The search browser fearture uses nearly only the browser.js file. That is why I propose to move the searchid generation from search.pl to browser.js. We then use Date.getTime() to use current timestamp as searchid, prefixed by 'scs_' like before. Test by using test plan of main patch : Too many search cursor cookies overflow HTTP-header size, when making multiple searches in the staff client
Two new patches need signoff : Bug 11369 - Correct usage of me.searchid in browser.js Bug 11369 - General searchid in browser.js
Created attachment 25729 [details] [review] Bug 11369 - Updating the jquery.cookie.js-plugin The current jquery.cookie-plugin crashes when trying to fetch all cookies using $.cookie(); Downloaded the newest plugin version and minified it. Now works as intended. Encountered an issue with the plugin now returning null when no cookies are found, and applied a fix in browser.js. ------------- - Test plan - ------------- Plugin is used in browser.js and batchMod.js so testing both Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 25730 [details] [review] Bug 11369 - Too many search cursor cookies overflow HTTP-header size, when making multiple searches in the staff client. To replicate this issue, make multiple searches in catalogue/search.pl. 50+ Should be enough to cause the HTTP-request header to overgrow. One can verify this issue by observing the searchCookie growth in browser's stored cookies. ------------- - TEST PLAN - ------------- Keep making searches. One should never have more than 10 searchCookies. Browser might display only 9, because for some reason the newest js-generated cookie is not included in Firefox's cookies listing. ------------ - DRAWBACK - ------------ Removing these cookies disables the search cursor for traversing search results (next/previous) for the removed cookie. This maybe be problematic in some cases, (for ex when multiple search tabs need to be open and they need to be traversed) One easy solution is to grow the amount of stored searchCookies from 10 to 20, but 10 is chosen so there will be plenty of room for other cookies as well. Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 25731 [details] [review] Bug 11369 - Correct usage of me.searchid in browser.js In browser.js, at creation of browser, the searchid is transmited to JS object into me.searchid. To be consistant, me.searchid should always be use, never searchid alone. In browseRecords function, setting searchid as parameter is useless because it is defined in me.searchid. Test with test plan of Bug 10404 Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 25732 [details] [review] Bug 11369 - General searchid in browser.js The search browser fearture uses nearly only the browser.js file. That is why I propose to move the searchid generation from search.pl to browser.js. We then use Date.getTime() to use current timestamp as searchid, prefixed by 'scs_' like before. Test by using test plan of main patch : Too many search cursor cookies overflow HTTP-header size, when making multiple searches in the staff client Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
In last patch, "General searchid in browser.js", i mean "Generate ...". :D
I think this patch can cause some problems, when you still have an old cookie in your browser. I had this error showing when testing batch modifying items: Error: Syntax error, unrecognized expression: :nth-child ...{href:function(a){return a.getAttribute("href")},type:function(a){return a.getAt... After restarting the browser, it's no problem. ... just noting - no blocker.
Created attachment 26501 [details] [review] [PASSED QA] Bug 11369 - Updating the jquery.cookie.js-plugin The current jquery.cookie-plugin crashes when trying to fetch all cookies using $.cookie(); Downloaded the newest plugin version and minified it. Now works as intended. Encountered an issue with the plugin now returning null when no cookies are found, and applied a fix in browser.js. ------------- - Test plan - ------------- Plugin is used in browser.js and batchMod.js so testing both Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Created attachment 26502 [details] [review] [PASSED QA] Bug 11369 - Too many search cursor cookies overflow HTTP-header size, when making multiple searches in the staff client. To replicate this issue, make multiple searches in catalogue/search.pl. 50+ Should be enough to cause the HTTP-request header to overgrow. One can verify this issue by observing the searchCookie growth in browser's stored cookies. ------------- - TEST PLAN - ------------- Keep making searches. One should never have more than 10 searchCookies. Browser might display only 9, because for some reason the newest js-generated cookie is not included in Firefox's cookies listing. ------------ - DRAWBACK - ------------ Removing these cookies disables the search cursor for traversing search results (next/previous) for the removed cookie. This maybe be problematic in some cases, (for ex when multiple search tabs need to be open and they need to be traversed) One easy solution is to grow the amount of stored searchCookies from 10 to 20, but 10 is chosen so there will be plenty of room for other cookies as well. Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Created attachment 26503 [details] [review] [PASSED QA] Bug 11369 - Correct usage of me.searchid in browser.js In browser.js, at creation of browser, the searchid is transmited to JS object into me.searchid. To be consistant, me.searchid should always be use, never searchid alone. In browseRecords function, setting searchid as parameter is useless because it is defined in me.searchid. Test with test plan of Bug 10404 Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Created attachment 26504 [details] [review] [PASSED QA] Bug 11369 - General searchid in browser.js The search browser feature uses nearly only the browser.js file. That is why I propose to move the searchid generation from search.pl to browser.js. We then use Date.getTime() to use current timestamp as searchid, prefixed by 'scs_' like before. Test by using test plan of main patch : Too many search cursor cookies overflow HTTP-header size, when making multiple searches in the staff client Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Passes tests and QA script. Tested the browse functionality in staff by using the different options (back to results, next, previous, etc) and the batch modifications for items. An old cookie can cause a Javascript error, but after restarting the browser/deleting cookies it all works correctly.
This works as described, at least if I haven't missed something. Like some of the other commenters I would feel a bit better about this if we were not losing history here at some point by limiting the number of cookies, but stored the data in another place - sessions or using the HTML5 feature.
Pushed to master. Thanks, Olli and Fridolin! In retrospect, the OPAC's version of results browsing that doesn't depend on cookies or retaining state seems preferable.
Pushed to 3.14.x, will be in 3.14.07
Sweet! I've encountered this problem a few times recently so it's nice to see it get remedied (in some way).
(In reply to Katrin Fischer from comment #22) > This works as described, at least if I haven't missed something. > > Like some of the other commenters I would feel a bit better about this if we > were not losing history here at some point by limiting the number of > cookies, but stored the data in another place - sessions or using the HTML5 > feature. I think using sessions is really the way to go. I'm going to backport this patch to our local Koha, but I still think it's less than ideal. I think the best way to do this is to keep using unique IDs for the search, and to store them and the relevant data in the active session in the database. In this way, we can still have multiple tabs open with unique searches and still use the record browser without the limits imposed by using cookies. At the moment, we store the OPAC record browser info in the active session in the database, but the limitation is that the key isn't unique (ie the key is "busc"). That means if you have multiple tabs open, the browser for all of them will return the results for the latest search... definitely not a good thing!
Adding reference to http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12295 for anyone adding patches manually...