Bug 7620 - OPACNoResult, add search string to available parameters
Summary: OPACNoResult, add search string to available parameters
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Koha Team Lyon 3
QA Contact: Jonathan Druart
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-02-29 13:36 UTC by Paul Poulain
Modified: 2013-05-23 06:23 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 7620 OPACNoResult, add search string to available parameters (1.48 KB, patch)
2012-02-29 13:54 UTC, Koha Team Lyon 3
Details | Diff | Splinter Review
OPACNoResults, add keyword to search string (2.07 KB, patch)
2012-03-30 09:15 UTC, Koha Team Lyon 3
Details | Diff | Splinter Review
OPACNoResult, Add keyword to query string (2.65 KB, patch)
2012-06-11 14:51 UTC, Katrin Fischer
Details | Diff | Splinter Review
OPACNoResultsFound, Add keyword to query string (2.16 KB, patch)
2012-06-12 16:55 UTC, Koha Team Lyon 3
Details | Diff | Splinter Review
OPACNoResult Add keyword to query string (2.18 KB, patch)
2012-10-11 14:26 UTC, Koha Team Lyon 3
Details | Diff | Splinter Review
[SIGNED-OFF] OPACNoResult, Add keyword to query string (2.23 KB, patch)
2012-12-21 15:41 UTC, Kyle M Hall
Details | Diff | Splinter Review
OPACNoResult, Add keyword to query string (2.30 KB, patch)
2012-12-21 15:50 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Paul Poulain 2012-02-29 13:36:53 UTC
When OPACNoResult contain something, it would be usefull to be able to have the search string entered by the patron
Comment 1 Koha Team Lyon 3 2012-02-29 13:54:07 UTC Comment hidden (obsolete)
Comment 2 Nicole C. Engard 2012-03-01 13:59:19 UTC
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
Comment 3 Jared Camins-Esakov 2012-03-10 18:10:28 UTC
I agree with Nicole. Using a literal string is inconsistent and just asking for trouble.
Comment 4 Koha Team Lyon 3 2012-03-30 09:15:12 UTC Comment hidden (obsolete)
Comment 5 Chris Cormack 2012-04-09 10:36:14 UTC
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.
Comment 6 Koha Team Lyon 3 2012-05-23 08:11:25 UTC
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 ?
Comment 7 Francois Charbonnier 2012-06-10 11:18:36 UTC
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
Comment 8 Katrin Fischer 2012-06-11 14:51:19 UTC Comment hidden (obsolete)
Comment 9 Koha Team Lyon 3 2012-06-11 16:35:07 UTC
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.
Comment 10 Koha Team Lyon 3 2012-06-12 16:55:10 UTC Comment hidden (obsolete)
Comment 11 Katrin Fischer 2012-06-12 17:59:00 UTC
I am sorry, but I think should be "needs signoff" for the improved patch. Thx for fixing it!
Comment 12 Koha Team Lyon 3 2012-06-13 07:53:41 UTC
Do you mean I declared "Signed Off" status instead of "Needs Signoff" ? If so, I'm sorry.
Comment 13 Jared Camins-Esakov 2012-07-07 15:39:13 UTC
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).
Comment 14 Koha Team Lyon 3 2012-07-09 07:53:45 UTC
(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 ?
Comment 15 Jared Camins-Esakov 2012-07-09 11:40:16 UTC
(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)
Comment 16 Koha Team Lyon 3 2012-07-09 12:08:56 UTC
(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) ?
Comment 17 Jared Camins-Esakov 2012-07-09 12:16:43 UTC
(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}.
Comment 18 Koha Team Lyon 3 2012-07-09 14:03:48 UTC
(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.
Comment 19 Katrin Fischer 2012-07-10 06:02:51 UTC
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}
Comment 20 Jonathan Druart 2012-08-22 14:21:39 UTC
"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}/ ).
Comment 21 Koha Team Lyon 3 2012-10-11 14:26:27 UTC Comment hidden (obsolete)
Comment 22 Kyle M Hall 2012-12-21 15:41:41 UTC Comment hidden (obsolete)
Comment 23 Jonathan Druart 2012-12-21 15:50:25 UTC
QA Comments:
Now it is ok for me.
Marked as Passed QA.
Comment 24 Jonathan Druart 2012-12-21 15:50:52 UTC
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>
Comment 25 Jared Camins-Esakov 2012-12-22 21:46:29 UTC
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)