Bug 38921 - Remove unused href from Cancel hold link
Summary: Remove unused href from Cancel hold link
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: Main
Hardware: All All
: P5 - low trivial
Assignee: Phil Ringnalda
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on: 34478
Blocks:
  Show dependency treegraph
 
Reported: 2025-01-17 22:22 UTC by Phil Ringnalda
Modified: 2025-04-22 20:28 UTC (History)
1 user (show)

See Also:
GIT URL:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
25.05.00,24.11.04
Circulation function:


Attachments
Bug 38921: Remove unused href from Cancel hold link (2.67 KB, patch)
2025-01-17 22:38 UTC, Phil Ringnalda
Details | Diff | Splinter Review
Bug 38921: Remove unused href from Cancel hold link (2.71 KB, patch)
2025-01-22 19:19 UTC, David Nind
Details | Diff | Splinter Review
Bug 38921: Remove unused href from Cancel hold link (2.81 KB, patch)
2025-01-24 07:39 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Phil Ringnalda 2025-01-17 22:22:34 UTC
We use koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc to display the list of existing holds in reserve/request.pl when you are placing a hold. The cancel button-links are JavaScript-only, showing a confirm modal and then POSTing a form from there, since the op is cud-cancel and has to be a POST.

But we left behind a pre-CSRF href attribute which is a double false-positive for me, since it has both op=cancel and uses the Template Toolkit html filter rather than the uri filter for things inserted in a URL parameter. You can see that it doesn't do anything by right-clicking and opening it in a new tab - there's no op named cancel, so it just displays the existing holds and lets you add one, repeating the page you opened the link from, only with garbage after the request.pl? in your URL.
Comment 1 Phil Ringnalda 2025-01-17 22:38:20 UTC
Created attachment 176766 [details] [review]
Bug 38921: Remove unused href from Cancel hold link

The trashcan icon to cancel a hold from the list of holds on a bib record has
a leftover href attribute from before the bug 34478 CSRF protection changes.
It has an 'op=cancel' which does nothing since there's only a cud-cancel op,
so it just falls through to the view case and redisplays the page if you open
it in a new tab, and doesn't get used at all if you click the trashcan, when
JavaScript builds up a form that you can POST.

Test plan:
1. Place a hold on any item.
2. On the list of holds, right-click the Cancel hold trashcan icon, and
   select Open in new tab
3. In the new tab, note that the URL in the browser address bar shows all
   sorts of things about op=cancel and borrowernumber and biblionumber and
   reserveid, but nothing changed, your hold wasn't deleted
4. Close that pointless tab, and apply patch
5. Reload the page with the list of holds, and again right-click the
   Cancel hold trashcan and select Open in new tab
6. In the new tab, note that the URL just has the biblionumber and a #, no
   other extraneous things
7. In the original tab, left-click the trashcan, confirm cancellation in the
   popup, and make sure that cancelling still does work.

Sponsored-by: Chetco Community Public Library
Comment 2 David Nind 2025-01-22 19:19:06 UTC
Created attachment 176926 [details] [review]
Bug 38921: Remove unused href from Cancel hold link

The trashcan icon to cancel a hold from the list of holds on a bib record has
a leftover href attribute from before the bug 34478 CSRF protection changes.
It has an 'op=cancel' which does nothing since there's only a cud-cancel op,
so it just falls through to the view case and redisplays the page if you open
it in a new tab, and doesn't get used at all if you click the trashcan, when
JavaScript builds up a form that you can POST.

Test plan:
1. Place a hold on any item.
2. On the list of holds, right-click the Cancel hold trashcan icon, and
   select Open in new tab
3. In the new tab, note that the URL in the browser address bar shows all
   sorts of things about op=cancel and borrowernumber and biblionumber and
   reserveid, but nothing changed, your hold wasn't deleted
4. Close that pointless tab, and apply patch
5. Reload the page with the list of holds, and again right-click the
   Cancel hold trashcan and select Open in new tab
6. In the new tab, note that the URL just has the biblionumber and a #, no
   other extraneous things
7. In the original tab, left-click the trashcan, confirm cancellation in the
   popup, and make sure that cancelling still does work.

Sponsored-by: Chetco Community Public Library
Signed-off-by: David Nind <david@davidnind.com>
Comment 3 Marcel de Rooy 2025-01-24 07:39:28 UTC
Created attachment 177099 [details] [review]
Bug 38921: Remove unused href from Cancel hold link

The trashcan icon to cancel a hold from the list of holds on a bib record has
a leftover href attribute from before the bug 34478 CSRF protection changes.
It has an 'op=cancel' which does nothing since there's only a cud-cancel op,
so it just falls through to the view case and redisplays the page if you open
it in a new tab, and doesn't get used at all if you click the trashcan, when
JavaScript builds up a form that you can POST.

Test plan:
1. Place a hold on any item.
2. On the list of holds, right-click the Cancel hold trashcan icon, and
   select Open in new tab
3. In the new tab, note that the URL in the browser address bar shows all
   sorts of things about op=cancel and borrowernumber and biblionumber and
   reserveid, but nothing changed, your hold wasn't deleted
4. Close that pointless tab, and apply patch
5. Reload the page with the list of holds, and again right-click the
   Cancel hold trashcan and select Open in new tab
6. In the new tab, note that the URL just has the biblionumber and a #, no
   other extraneous things
7. In the original tab, left-click the trashcan, confirm cancellation in the
   popup, and make sure that cancelling still does work.

Sponsored-by: Chetco Community Public Library
Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 4 Katrin Fischer 2025-01-24 11:03:48 UTC
Pushed for 25.05!

Well done everyone, thank you!
Comment 5 Paul Derscheid 2025-04-22 20:28:09 UTC
Nice work everyone!

Pushed to 24.11.x for 24.11.04