Summary: | Self-checkouts restart every second if invalid data in timeout sysprefs | ||
---|---|---|---|
Product: | Koha | Reporter: | David Cook <dcook> |
Component: | Self checkout | Assignee: | Bugs List <koha-bugs> |
Status: | RESOLVED INVALID | QA Contact: | Testopia <testopia> |
Severity: | blocker | ||
Priority: | P5 - low | CC: | david, emmi.takkinen, jonathan.druart, lucas |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
GIT URL: | Change sponsored?: | --- | |
Patch complexity: | --- | Documentation contact: | |
Documentation submission: | Text to go in the release notes: | ||
Version(s) released in: | Circulation function: | ||
Bug Depends on: | 36586 | ||
Bug Blocks: | |||
Attachments: | Bug 40336: Validate timeout syspref for self-checks |
Description
David Cook
2025-07-10 00:19:36 UTC
Created attachment 183910 [details] [review] Bug 40336: Validate timeout syspref for self-checks This patch validates the numbers passed to the self-check timeout code, so that it must be a valid number >= 10 seconds. If it fails this validation, it defaults to 120 seconds. Test plan: 0. Apply the patch 1. Enable the SelfCheckInModule and WebBasedSelfCheck syspref 2. Try out these modules using varying values in the SelfCheckInTimeout and SelfCheckTimeout system preferences resepectives. For instance: - A blank space (ie " ") - A negative number (e.g. -7) - An alphabetic string (e.g. aa) - A positive number less than 10 (e.g. 5) - A positive number greater than 10 (e.g. 60) 3. Check the "idle_timeout" value set in the timer objects using the following in the browser console: console.log(window.sci_login_timer); console.log(window.sco_login_timer); 4. Note that all cases except positive numbers greater than or equal to 10 get reset to 120 Tested this on SCO and all values expect positive 10 or greater indeed reset to 120. But I'm still seeing constant restarting even with these values. So patch does work as intended but doesn't solve the whole problem. I'll try to set up a SCI on my environment and test that too just to be sure. Tested now with SCI, same results. It would be good to have systempreferences.type supports "integer" and have such problems caught before (during the input). Sorry David, we've decided to revert Bug 36586 for now. (In reply to Emmi Takkinen from comment #2) > Tested this on SCO and all values expect positive 10 or greater indeed reset > to 120. But I'm still seeing constant restarting even with these values. So > patch does work as intended but doesn't solve the whole problem. I'll try to > set up a SCI on my environment and test that too just to be sure. Do you have an Internet accessible Koha that can reproduce this problem? I could take a look there. Otherwise, I think I've troubleshooted this as far as I can. (In reply to Jonathan Druart from comment #4) > It would be good to have systempreferences.type supports "integer" and have > such problems caught before (during the input). Agreed. This would be a good use case for adding syspref validation methods. (In reply to Lucas Gass (lukeg) from comment #5) > Sorry David, we've decided to revert Bug 36586 for now. No worries. Did you try out the patch or was it a strategic decision? Based off Emmi's experience, it looks like there could be something else going on there in any case. Locally, we are suspending CSRF for self-check for older versions without bug 40108. I guess we'll just keep doing that until some other fix is done. We recently had an experience where a library's PC catalogue was resubmitting the opac-search.pl every second which was interesting. Library staff suspect a glitch with the kiosk software (rather than Koha) but who knows. (In reply to David Cook from comment #8) > Locally, we are suspending CSRF for self-check for older versions without > bug 40108. I guess we'll just keep doing that until some other fix is done. Sorry I meant bug 36586 (In reply to David Cook from comment #6) > (In reply to Emmi Takkinen from comment #2) > > Tested this on SCO and all values expect positive 10 or greater indeed reset > > to 120. But I'm still seeing constant restarting even with these values. So > > patch does work as intended but doesn't solve the whole problem. I'll try to > > set up a SCI on my environment and test that too just to be sure. > > Do you have an Internet accessible Koha that can reproduce this problem? I > could take a look there. > > Otherwise, I think I've troubleshooted this as far as I can. All our Koha instances have restricted access. But I can try to take a look at this and see if what I can find. David, you were on the right track with this. I ran code from bug 36586 through Copilot and it pointed out that variable const idle_timeout has to be converted as number also in function login_timeout found from sci-main.tt and sco-main.tt: const idleTimeout = Number("[% Koha.Preference('SelfCheckTimeout') | html %]"); This stopped the constant refreshing. Unfortunately with this bugs patch in place converting no longer helps no matter how many Number() methods I add :D (In reply to Emmi Takkinen from comment #11) > David, you were on the right track with this. I ran code from bug 36586 > through Copilot and it pointed out that variable const idle_timeout has to > be converted as number also in function login_timeout found from sci-main.tt > and sco-main.tt: const idleTimeout = Number("[% > Koha.Preference('SelfCheckTimeout') | html %]"); This stopped the constant > refreshing. Unfortunately with this bugs patch in place converting no longer > helps no matter how many Number() methods I add :D Sorry I'm very confused about what you're trying to say here. It sounds like you're saying that the patch works, but that it also does not work? I suppose it doesn't really matter anyway since bug 36586 has been reverted. |