Summary: | OPACNoResult, add search string to available parameters | ||
---|---|---|---|
Product: | Koha | Reporter: | Paul Poulain <paul.poulain> |
Component: | OPAC | Assignee: | Koha Team University Lyon 3 <koha> |
Status: | CLOSED FIXED | QA Contact: | Jonathan Druart <jonathan.druart> |
Severity: | enhancement | ||
Priority: | P5 - low | CC: | chris, francois.charbonnier, jcamins, jonathan.druart, kyle, nengard |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | --- |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: | ||
Circulation function: | |||
Bug Depends on: | |||
Bug Blocks: | 28241 | ||
Attachments: |
Bug 7620 OPACNoResult, add search string to available parameters
OPACNoResults, add keyword to search string OPACNoResult, Add keyword to query string OPACNoResultsFound, Add keyword to query string OPACNoResult Add keyword to query string [SIGNED-OFF] OPACNoResult, Add keyword to query string OPACNoResult, Add keyword to query string |
Description
Paul Poulain
2012-02-29 13:36:53 UTC
Created attachment 7941 [details] [review] Bug 7620 OPACNoResult, add search string to available parameters This patch add the string entered by the user to OPACNoResultFound Test plan: * activate OPACNoResultFound and use "query_kw" somewhere. For example : <a href="www.google.com?q=query_kw">Search on Google</a> * try a search that return no result * you should see the link to search the query on google I have not tested this yet, but we should probably follow the style set forth in other preferences so instead of "query_kw" it should be "{query_kw}" maybe even in all caps. That's how it's done on the other preferences where we enter placeholders. Nicole I agree with Nicole. Using a literal string is inconsistent and just asking for trouble. Created attachment 8713 [details] [review] OPACNoResults, add keyword to search string The same patch redone with place holder I like the idea of this patch, but unfortunately it adds an xss vulnerability.
For example, if I searched on
><script type="text/javascript" src="http://link/to/evil.js"></script> that would be substituted and output (and run).
OPACNoResult is not piped through the html filter, because then it couldn't have links in it, so it's not a simple fix to just change that.
Probably the easiest fix is to run the $query_kw through HTML::Scrubber before substituting it in the syspref.
Well, I try to see how to proceed with C4::Scrubber and how it had been already used in other koha scripts like opac-tags.pl but I don't really get how to adapt it to the present case. However, if it's a question of sanitizing the $query_kw string before substitution, you 'll note that the regexp used to extract keywords does it since it keeps only alphanumeric characters (plus -): To pick your example, it goes like this : my $query_kw='<script type="text/javascript" src="http://link/to/evil.js"></script>'; then after theese lines : my @query_kw=($query_kw=~ /([-\w]+\b)(?:[^,:]|$)/g); my $query_kw=join('+',@query_kw); the output to be substituted will be : my $query_kw='script+type+text+javascript+src+link+to+evil+js+script' may be, that could do the trick anyway ? The patch does not apply : OPACNoResults, add keyword to search string Apply? [yn] y Applying: OPACNoResult, Add keyword to query string Using index info to reconstruct a base tree... Falling back to patching base and 3-way merge... Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref Auto-merging opac/opac-search.pl CONFLICT (content): Merge conflict in opac/opac-search.pl Failed to merge in the changes. Patch failed at 0001 OPACNoResult, Add keyword to query string When you have resolved this problem run "git am --resolved". If you would prefer to skip this patch, instead run "git am --skip". To restore the original branch and stop patching run "git am --abort". Patch left in /tmp/OPACNoResults-add-keyword-to-search-string-cfak79.patch Created attachment 10219 [details] [review] OPACNoResult, Add keyword to query string This patch add the keywords to the string entered by the user to OPACNoResultFound Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Rebased 2012-06-11 Note: Patch does not work for search terms containing umlauts like äüö. Those characters are lost from the displayed search term. Ok,I think I see the reason why now : The \w class can match accentuated characters (as it does at lyon3) but unfortunately only if the adequate set of locales is installed on the system. Created attachment 10253 [details] [review] OPACNoResultsFound, Add keyword to query string Further investigations showed that was not a question of \w class with locales but an encoding problem on the keywords extracted I am sorry, but I think should be "needs signoff" for the improved patch. Thx for fixing it! Do you mean I declared "Signed Off" status instead of "Needs Signoff" ? If so, I'm sorry. I just tested, and this patch does not work for me. Adding {QUERY_KW} to OPACNoResultsFound does not change the output at all (without this patch, the text {QUERY_KW} showed up). (In reply to comment #13) > I just tested, and this patch does not work for me. Adding {QUERY_KW} to > OPACNoResultsFound does not change the output at all (without this patch, > the text {QUERY_KW} showed up). strange... it works perfectly on our system, could you mention the query you had tested in OPACNoResultsFound and the keywords used ? (In reply to comment #14) > (In reply to comment #13) > > I just tested, and this patch does not work for me. Adding {QUERY_KW} to > > OPACNoResultsFound does not change the output at all (without this patch, > > the text {QUERY_KW} showed up). > > strange... it works perfectly on our system, could you mention the query you > had tested in OPACNoResultsFound and the keywords used ? asflfsjsfja (no, that was not a typo) (In reply to comment #15) > (In reply to comment #14) > > (In reply to comment #13) > > > I just tested, and this patch does not work for me. Adding {QUERY_KW} to > > > OPACNoResultsFound does not change the output at all (without this patch, > > > the text {QUERY_KW} showed up). > > > > strange... it works perfectly on our system, could you mention the query you > > had tested in OPACNoResultsFound and the keywords used ? > > asflfsjsfja (no, that was not a typo) Thanks for the keyword but what was your full query (the one you put in OPACNoResultsFound) ? (In reply to comment #16) > (In reply to comment #15) > > (In reply to comment #14) > > > (In reply to comment #13) > > > > I just tested, and this patch does not work for me. Adding {QUERY_KW} to > > > > OPACNoResultsFound does not change the output at all (without this patch, > > > > the text {QUERY_KW} showed up). > > > > > > strange... it works perfectly on our system, could you mention the query you > > > had tested in OPACNoResultsFound and the keywords used ? > > > > asflfsjsfja (no, that was not a typo) > > Thanks for the keyword but what was your full query (the one you put in > OPACNoResultsFound) ? Sorry, I thought you meant the query I entered into Koha. I tried it with the following in OPACNoResultsFound: Search elsewhere: <a href="www.google.com/?q={QUERY_KW}">Google</a> for {QUERY_KW}. (In reply to comment #15) > (In reply to comment #14) > > (In reply to comment #13) > > > I just tested, and this patch does not work for me. Adding {QUERY_KW} to > > > OPACNoResultsFound does not change the output at all (without this patch, > > > the text {QUERY_KW} showed up). > > > > strange... it works perfectly on our system, could you mention the query you > > had tested in OPACNoResultsFound and the keywords used ? > > asflfsjsfja (no, that was not a typo) Thanks for the keyword but what was your full query (the one you put in OPACNoResultsFound) ?(In reply to comment #17) > (In reply to comment #16) > > (In reply to comment #15) > > > (In reply to comment #14) > > > > (In reply to comment #13) > > > > > I just tested, and this patch does not work for me. Adding {QUERY_KW} to > > > > > OPACNoResultsFound does not change the output at all (without this patch, > > > > > the text {QUERY_KW} showed up). > > > > > > > > strange... it works perfectly on our system, could you mention the query you > > > > had tested in OPACNoResultsFound and the keywords used ? > > > > > > asflfsjsfja (no, that was not a typo) > > > > Thanks for the keyword but what was your full query (the one you put in > > OPACNoResultsFound) ? > > Sorry, I thought you meant the query I entered into Koha. I tried it with > the following in OPACNoResultsFound: Search elsewhere: <a > href="www.google.com/?q={QUERY_KW}">Google</a> for {QUERY_KW}. Your url and keyword works on our system, the substitution is done : www.google.com/?q=asflfsjsfja I have no idea why it doesn't on your system. But I noted another point to be fixed : without the http mention in href attribute (href="http://www.google...) the address is searched in the web directory of koha. Hi, I retested this on my system and it works nicely - could it be some kind of configuration issue? I tried: - simple keyword searches - searches from the advanced search page with different indexes - Umlaut searches The content of my system preference is: Something {QUERY_KW} "It works for me" But (qa comment): it is useless to generate $query_kw if the syspref does not match {QUERY_KW}, please use if( $var =~ m/{QUERY_KW}/ ). Created attachment 12773 [details] [review] OPACNoResult Add keyword to query string patch redone according to your suggestion Created attachment 14236 [details] [review] [SIGNED-OFF] OPACNoResult, Add keyword to query string When search failed, this patch gets the keyword(s) and add them to the string entered by the user to OPACNoResultFound http://bugs.koha-community.org/show_bug.cgi?id=7620 Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Tested by setting OPACNoResultsFound to 'Search elsewhere: <a href="http://www.google.com/?q={QUERY_KW}">Google</a> for {QUERY_KW}' QA Comments: Now it is ok for me. Marked as Passed QA. Created attachment 14240 [details] [review] OPACNoResult, Add keyword to query string When search failed, this patch gets the keyword(s) and add them to the string entered by the user to OPACNoResultFound http://bugs.koha-community.org/show_bug.cgi?id=7620 Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> This patch has been pushed to master. As a reminder for future patches, please be sure to include the bug number in the patch subject (first line of the commit message) |