At the moment, the "marclist" parameter - used in the "Where" field for the OPAC Authority search - isn't being properly validated. This can lead to syntactically incorrect queries which cause server errors.
To reproduce: 1. Go to /cgi-bin/koha/opac-authorities-home.pl 2. Type "test" into "Term(s)" field 3. Modify the selected option for the "marclist" parameter for "Where" field to say "this is broken" 4. Click "Submit" 5. Note that a server error is generated
Created attachment 159856 [details] [review] Bug 35578: Validate "Where" in OPAC Authority search This patch adds validation to the "Where" field in OPAC Authority search. Test plan: 0. Apply the patch and koha-plack --reload kohadev 1. Go to http://localhost:8080/cgi-bin/koha/opac-authorities-home.pl 2. Type "test" into "Term(s)" field 3. Click "Submit" 4. Confirm a result is found 5. Repeat the above using "Where" values of "in the complete record", "in any heading", and "in main entry" 6. Using the HTML inspector in the browser, change the value of the selected option of the "marclist" select element to "this is broken" 7. Click "Submit" 8. Confirm a result is found (ie it's not throwing a fatal error anymore)
Created attachment 159926 [details] [review] Bug 35578: Validate "Where" in OPAC Authority search This patch adds validation to the "Where" field in OPAC Authority search. Test plan: 0. Apply the patch and koha-plack --reload kohadev 1. Go to http://localhost:8080/cgi-bin/koha/opac-authorities-home.pl 2. Type "test" into "Term(s)" field 3. Click "Submit" 4. Confirm a result is found 5. Repeat the above using "Where" values of "in the complete record", "in any heading", and "in main entry" 6. Using the HTML inspector in the browser, change the value of the selected option of the "marclist" select element to "this is broken" 7. Click "Submit" 8. Confirm a result is found (ie it's not throwing a fatal error anymore) Signed-off-by: David Nind <david@davidnind.com>
Why do we call multi_param for marclist ? Is there any case where it makes sense to have multiple values ?
(In reply to Julian Maurice from comment #4) > Why do we call multi_param for marclist ? Is there any case where it makes > sense to have multiple values ? I've looked at ./koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-authorities-home.tt and opac/opac-authorities-home.pl. At a glance, it's not clear why these are array variables. I've done a bit of "git blame" and basically it looks like really old legacy code that basically never made sense. I think it's out of scope for this bug, but this could certainly be refactored.
Created attachment 160984 [details] [review] Bug 35578: Validate "Where" in OPAC Authority search This patch adds validation to the "Where" field in OPAC Authority search. Test plan: 0. Apply the patch and koha-plack --reload kohadev 1. Go to http://localhost:8080/cgi-bin/koha/opac-authorities-home.pl 2. Type "test" into "Term(s)" field 3. Click "Submit" 4. Confirm a result is found 5. Repeat the above using "Where" values of "in the complete record", "in any heading", and "in main entry" 6. Using the HTML inspector in the browser, change the value of the selected option of the "marclist" select element to "this is broken" 7. Click "Submit" 8. Confirm a result is found (ie it's not throwing a fatal error anymore) Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 160985 [details] [review] Bug 35578: Simplify the code using intersect Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 161235 [details] [review] Bug 35578: Validate "Where" in OPAC Authority search This patch adds validation to the "Where" field in OPAC Authority search. Test plan: 0. Apply the patch and koha-plack --reload kohadev 1. Go to http://localhost:8080/cgi-bin/koha/opac-authorities-home.pl 2. Type "test" into "Term(s)" field 3. Click "Submit" 4. Confirm a result is found 5. Repeat the above using "Where" values of "in the complete record", "in any heading", and "in main entry" 6. Using the HTML inspector in the browser, change the value of the selected option of the "marclist" select element to "this is broken" 7. Click "Submit" 8. Confirm a result is found (ie it's not throwing a fatal error anymore) Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 161236 [details] [review] Bug 35578: Simplify the code using intersect Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Trivial fix, agree the refactoring/cleanup can go on another bug. Passing QA
Pushed for 24.05! Well done everyone, thank you!
Pushed to 23.11.x for 23.11.03