Bug 28885 - OpacBrowseResults can cause errors with bad search indexes
Summary: OpacBrowseResults can cause errors with bad search indexes
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: Main
Hardware: All All
: P5 - low major
Assignee: Kyle M Hall (khall)
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-08-19 14:49 UTC by Kyle M Hall (khall)
Modified: 2022-06-06 20:27 UTC (History)
2 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
21.11.00,21.05.04,20.11.10,20.05.16
Circulation function:


Attachments
Bug 28885: Skip invalid biblios for OpacBrowseResults (1.97 KB, patch)
2021-08-19 14:56 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 28885: Skip invalid biblios for OpacBrowseResults (2.00 KB, patch)
2021-08-22 20:51 UTC, David Nind
Details | Diff | Splinter Review
Bug 28885: Skip invalid biblios for OpacBrowseResults (2.15 KB, patch)
2021-08-30 18:32 UTC, Joonas Kylmälä
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall (khall) 2021-08-19 14:49:10 UTC
If a record is deleted from Koha, but is for some reason not deleted from the search indexes, OpacBrowseResults can cause an ISE if the deleted record is in the search results for any given item. OpacBrowseResults loops through the search results, and checks if there is a biblionumber, but does *not* check to see if a result was pulled from the database for that biblionumber. It simply assumes the result must exist.

We should be checking to ensure the biblionumber was valid before operating on the biblio object.
Comment 1 Kyle M Hall (khall) 2021-08-19 14:56:42 UTC
Created attachment 123978 [details] [review]
Bug 28885: Skip invalid biblios for OpacBrowseResults

If a record is deleted from Koha, but is for some reason not deleted from the search indexes, OpacBrowseResults can cause an ISE if the deleted record is in the search results for any given item. OpacBrowseResults loops through the search results, and checks if there is a biblionumber, but does *not* check to see if a result was pulled from the database for that biblionumber. It simply assumes the result must exist.

We should be checking to ensure the biblionumber was valid before operating on the biblio object.

Test Plan:
1) Use zebra for searching
2) Disable koha-indexer
3) Enable OpacBrowseResults
4) Perform a search
5) Delete an item in the search results
6) View on of the remaining items in the search results
7) Note the error
8) Apply this patch
9) Restart plack
10) Reload the page
11) The error should be gone!
Comment 2 David Nind 2021-08-22 20:51:22 UTC
Created attachment 124009 [details] [review]
Bug 28885: Skip invalid biblios for OpacBrowseResults

If a record is deleted from Koha, but is for some reason not deleted from the search indexes, OpacBrowseResults can cause an ISE if the deleted record is in the search results for any given item. OpacBrowseResults loops through the search results, and checks if there is a biblionumber, but does *not* check to see if a result was pulled from the database for that biblionumber. It simply assumes the result must exist.

We should be checking to ensure the biblionumber was valid before operating on the biblio object.

Test Plan:
1) Use zebra for searching
2) Disable koha-indexer
3) Enable OpacBrowseResults
4) Perform a search
5) Delete an item in the search results
6) View on of the remaining items in the search results
7) Note the error
8) Apply this patch
9) Restart plack
10) Reload the page
11) The error should be gone!

Signed-off-by: David Nind <david@davidnind.com>
Comment 3 Joonas Kylmälä 2021-08-30 18:32:38 UTC
Created attachment 124252 [details] [review]
Bug 28885: Skip invalid biblios for OpacBrowseResults

If a record is deleted from Koha, but is for some reason not deleted from the search indexes, OpacBrowseResults can cause an ISE if the deleted record is in the search results for any given item. OpacBrowseResults loops through the search results, and checks if there is a biblionumber, but does *not* check to see if a result was pulled from the database for that biblionumber. It simply assumes the result must exist.

We should be checking to ensure the biblionumber was valid before operating on the biblio object.

Test Plan:
1) Use zebra for searching
2) Disable koha-indexer
3) Enable OpacBrowseResults
4) Perform a search
5) Delete an item in the search results
6) View on of the remaining items in the search results
7) Note the error
8) Apply this patch
9) Restart plack
10) Reload the page
11) The error should be gone!

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>
Comment 4 Jonathan Druart 2021-08-31 08:28:44 UTC
Pushed to master for 21.11, thanks to everybody involved!
Comment 5 Kyle M Hall (khall) 2021-09-03 15:05:05 UTC
Pushed to 21.05.x for 21.05.04
Comment 6 Fridolin Somers 2021-09-08 20:38:36 UTC
Pushed to 20.11.x for 20.11.10
Comment 7 Victor Grousset/tuxayo 2021-09-20 11:53:17 UTC
Backported: Pushed to 20.05.x branch for 20.05.16