Bug 35578

Summary: Validate "Where" in OPAC Authority search
Product: Koha Reporter: David Cook <dcook>
Component: OPACAssignee: David Cook <dcook>
Status: RESOLVED FIXED QA Contact: Martin Renvoize (ashimema) <martin.renvoize>
Severity: minor    
Priority: P5 - low CC: fridolin.somers, julian.maurice, lucas
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35566
GIT URL: Change sponsored?: ---
Patch complexity: Small patch Documentation contact:
Documentation submission: Text to go in the release notes:
Version(s) released in:
24.05.00,23.11.03
Circulation function:
Attachments: Bug 35578: Validate "Where" in OPAC Authority search
Bug 35578: Validate "Where" in OPAC Authority search
Bug 35578: Validate "Where" in OPAC Authority search
Bug 35578: Simplify the code using intersect
Bug 35578: Validate "Where" in OPAC Authority search
Bug 35578: Simplify the code using intersect

Description David Cook 2023-12-15 00:08:01 UTC
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.
Comment 1 David Cook 2023-12-15 00:09:28 UTC
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
Comment 2 David Cook 2023-12-15 00:17:09 UTC
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)
Comment 3 David Nind 2023-12-16 20:48:48 UTC
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>
Comment 4 Julian Maurice 2023-12-21 13:41:46 UTC
Why do we call multi_param for marclist ? Is there any case where it makes sense to have multiple values ?
Comment 5 David Cook 2024-01-02 01:22:15 UTC
(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.
Comment 6 Jonathan Druart 2024-01-12 15:55:17 UTC
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>
Comment 7 Jonathan Druart 2024-01-12 15:55:19 UTC
Created attachment 160985 [details] [review]
Bug 35578: Simplify the code using intersect

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 8 Martin Renvoize (ashimema) 2024-01-22 12:40:09 UTC
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>
Comment 9 Martin Renvoize (ashimema) 2024-01-22 12:40:12 UTC
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>
Comment 10 Martin Renvoize (ashimema) 2024-01-22 12:40:39 UTC
Trivial fix, agree the refactoring/cleanup can go on another bug.

Passing QA
Comment 11 Katrin Fischer 2024-01-29 11:28:56 UTC
Pushed for 24.05!

Well done everyone, thank you!
Comment 12 Fridolin Somers 2024-02-01 14:51:35 UTC
Pushed to 23.11.x for 23.11.03