Bug 39262 - showCartUpdate indirectly uses eval()
Summary: showCartUpdate indirectly uses eval()
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor
Assignee: Owen Leonard
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2025-03-06 06:11 UTC by David Cook
Modified: 2025-03-17 07:28 UTC (History)
1 user (show)

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


Attachments
Bug 39262: Don't pass string literals to setTimeout (3.39 KB, patch)
2025-03-07 13:09 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 39262: Don't pass string literals to setTimeout (3.43 KB, patch)
2025-03-11 05:09 UTC, David Cook
Details | Diff | Splinter Review
Bug 39262: Don't pass string literals to setTimeout (3.53 KB, patch)
2025-03-14 10:50 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 David Cook 2025-03-06 06:11:24 UTC
In ./koha-tmpl/opac-tmpl/bootstrap/js/basket.js there is a function "showCartUpdate" which calls the following line:

setTimeout("hideCart()", 2000);

When you pass a string as the first argument to setTimeout, it performs an eval(). While I don't think there's any security risk in this, it's suboptimal, and it will cause problems when we try to use Content-Security-Policy later.

We should just update this to the following:

setTimeout(hideCart, 2000);

--

There's probably other examples of this, but this is the one that jumped out to me while working on CSP.
Comment 1 Owen Leonard 2025-03-07 13:09:34 UTC
Created attachment 179078 [details] [review]
Bug 39262: Don't pass string literals to setTimeout

This patch updates a few places in our code where we pass string
literals to JavaScript's setTimeout() function. This is not recommended:
https://developer.mozilla.org/en-US/docs/Web/API/Window/setTimeout#passing_string_literals

To test, apply the patch and clear your browser cache if necessary.

- In the OPAC, perform a catalog search. From the search results page,
  test adding some items to the cart. The "Item has been added to your
  cart" message should appear and disappear correctly.
- In the staff interface, view the contents of a list. Click the "Print
  list" button in the toolbar. A new tab should load and trigger the
  browser's print window. Printing the page or cancelling should
  automatically close the tab.
- Go to Acquisitions -> Vendor -> Basket groups. In the "Open" tab,
  click the "Close and export as PDF" button. This should trigger
  a page reload and the creation of a PDF. The basket group should now
  appear under the "Closed" tab.

Sponsored-by: Athens County Public Libraries
Comment 2 David Cook 2025-03-11 05:09:30 UTC
Created attachment 179137 [details] [review]
Bug 39262: Don't pass string literals to setTimeout

This patch updates a few places in our code where we pass string
literals to JavaScript's setTimeout() function. This is not recommended:
https://developer.mozilla.org/en-US/docs/Web/API/Window/setTimeout#passing_string_literals

To test, apply the patch and clear your browser cache if necessary.

- In the OPAC, perform a catalog search. From the search results page,
  test adding some items to the cart. The "Item has been added to your
  cart" message should appear and disappear correctly.
- In the staff interface, view the contents of a list. Click the "Print
  list" button in the toolbar. A new tab should load and trigger the
  browser's print window. Printing the page or cancelling should
  automatically close the tab.
- Go to Acquisitions -> Vendor -> Basket groups. In the "Open" tab,
  click the "Close and export as PDF" button. This should trigger
  a page reload and the creation of a PDF. The basket group should now
  appear under the "Closed" tab.

Sponsored-by: Athens County Public Libraries
Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 3 David Cook 2025-03-11 05:09:57 UTC
Thanks for taking care of these, Owen. Much appreciated.
Comment 4 Marcel de Rooy 2025-03-14 10:50:34 UTC
Created attachment 179321 [details] [review]
Bug 39262: Don't pass string literals to setTimeout

This patch updates a few places in our code where we pass string
literals to JavaScript's setTimeout() function. This is not recommended:
https://developer.mozilla.org/en-US/docs/Web/API/Window/setTimeout#passing_string_literals

To test, apply the patch and clear your browser cache if necessary.

- In the OPAC, perform a catalog search. From the search results page,
  test adding some items to the cart. The "Item has been added to your
  cart" message should appear and disappear correctly.
- In the staff interface, view the contents of a list. Click the "Print
  list" button in the toolbar. A new tab should load and trigger the
  browser's print window. Printing the page or cancelling should
  automatically close the tab.
- Go to Acquisitions -> Vendor -> Basket groups. In the "Open" tab,
  click the "Close and export as PDF" button. This should trigger
  a page reload and the creation of a PDF. The basket group should now
  appear under the "Closed" tab.

Sponsored-by: Athens County Public Libraries
Signed-off-by: David Cook <dcook@prosentient.com.au>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 5 Katrin Fischer 2025-03-17 07:28:39 UTC
Pushed for 25.05!

Well done everyone, thank you!