Prior to the CSRF protection, deleting an authority record from search results built up a URL for a GET, and used the uri TT filter on the value param, which is where the search term you used to find the record gets passed to the script for reloading afterward. That was fine (though the use of the html filter on the other params was a bit sketchy, given that they were being stuffed into a URL, not displayed as HTML), but when https://git.koha-community.org/Koha-community/Koha/commit/62d9f73e9e3bfe7fbca1340c753b75c951aceaf1 switched from a GET to a form that POSTs, it retained the same escaping, so now we have a URI encoded value being put into a form input, passed over to the script which certainly isn't expecting that, and then stuffed back into the search box and searched for literally: if you do an authority search for "professional computing" and delete one of the results, the page that loads after the deletion is a search for "professional%20computing"
Created attachment 172292 [details] [review] Bug 38056: Search term after deleting an authority shouldn't be URI encoded Now that deleting an authority record uses a form, rather than building up a URL in JavaScript, using the filter | uri on the value (confusingly named value) which passes along the search terms to reload after deleting results in a bad search. It should instead use the filter | html. Test plan: 1. Without the patch, top menu - Authorities 2. The default search, Main heading ($a only), Authority type Default, Operator contains, Order by Heading A-Z will work fine. You need to a search which will return more than one of the same thing (not hard in ktd) - for MARC21 search for a professional 3. You should get five results. For one of them, Actions - Delete - Confirm that you meant to click it 4. You now have a%20professional in the search box, and search results for whatever ElasticSearch thinks a% means. It's certainly not professional - you can change that term to any random string and get the same results 5. Apply patch, top menu - Authorities 6. Repeat steps 2 and 3, but this time after deleting another result, you will get a professional without the %20 in the search box, and your results will be the remaining Addison-Wesley professional computing series authorities Sponsored-by: Chetco Community Public Library
Created attachment 173009 [details] [review] Bug 38056: Search term after deleting an authority shouldn't be URI encoded Now that deleting an authority record uses a form, rather than building up a URL in JavaScript, using the filter | uri on the value (confusingly named value) which passes along the search terms to reload after deleting results in a bad search. It should instead use the filter | html. Test plan: 1. Without the patch, top menu - Authorities 2. The default search, Main heading ($a only), Authority type Default, Operator contains, Order by Heading A-Z will work fine. You need to a search which will return more than one of the same thing (not hard in ktd) - for MARC21 search for a professional 3. You should get five results. For one of them, Actions - Delete - Confirm that you meant to click it 4. You now have a%20professional in the search box, and search results for whatever ElasticSearch thinks a% means. It's certainly not professional - you can change that term to any random string and get the same results 5. Apply patch, top menu - Authorities 6. Repeat steps 2 and 3, but this time after deleting another result, you will get a professional without the %20 in the search box, and your results will be the remaining Addison-Wesley professional computing series authorities Sponsored-by: Chetco Community Public Library Signed-off-by: David Nind <david@davidnind.com>
Created attachment 173017 [details] [review] Bug 38056: Search term after deleting an authority shouldn't be URI encoded Now that deleting an authority record uses a form, rather than building up a URL in JavaScript, using the filter | uri on the value (confusingly named value) which passes along the search terms to reload after deleting results in a bad search. It should instead use the filter | html. Test plan: 1. Without the patch, top menu - Authorities 2. The default search, Main heading ($a only), Authority type Default, Operator contains, Order by Heading A-Z will work fine. You need to a search which will return more than one of the same thing (not hard in ktd) - for MARC21 search for a professional 3. You should get five results. For one of them, Actions - Delete - Confirm that you meant to click it 4. You now have a%20professional in the search box, and search results for whatever ElasticSearch thinks a% means. It's certainly not professional - you can change that term to any random string and get the same results 5. Apply patch, top menu - Authorities 6. Repeat steps 2 and 3, but this time after deleting another result, you will get a professional without the %20 in the search box, and your results will be the remaining Addison-Wesley professional computing series authorities Sponsored-by: Chetco Community Public Library Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Pushed for 24.11! Well done everyone, thank you!
Backported to 24.05.x for upcoming 24.05.05
Depends on CSRF not in 23.11, and we for sure never documented what it fixed.