|
Description
victor
2023-06-28 14:03:02 UTC
Thanks Victor for reporting this one. Moving this from enh to bug. Starting with normal, but we could raise severity still. Created attachment 152807 [details] [review] Bug 34146: Do not allow multiple copies to crash server Currently hardcoded to 1000. Can be refined later. Let's first prevent this kind of accidents. Test plan: Add additem.pl. Edit the 1000 to 2. Restart all. Add 3 multiple copies. Notice that you got 2. Revert your code change. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Trivial fix, self signoff. Raising to major Created attachment 152808 [details] [review] Bug 34146: Do not allow multiple copies to crash server Currently hardcoded to 1000. Can be refined later. Let's first prevent this kind of accidents. Test plan: Add additem.pl. Edit the 1000 to 2. Restart all. Add 3 multiple copies. Notice that you got 2. Revert your code change. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Someone removed this btw in js:
+ <!-- Add a soft-limit of 99 with a reminder about potential data entry error -->
+ if (f>99) {
+ return confirm(_("More than 99 copies will be added. Should they be added?"));
It seems that Bootstrap does not respect the size attribute that is present on the number_of_copies input. There is a size="2" but it does not work. (In reply to Marcel de Rooy from comment #7) > There is a size="2" but it does not work. It sounds like you may be thinking of the "maxlength" attribute? Size only controls the width, not the values it accepts. (In reply to Owen Leonard from comment #8) > (In reply to Marcel de Rooy from comment #7) > > There is a size="2" but it does not work. > > It sounds like you may be thinking of the "maxlength" attribute? Size only > controls the width, not the values it accepts. Good point. Will add maxlength. Was trying to deduct why the check was removed. Note that size for a input text has a vague connection to the number of chars according to html specs. This attribute tells the user agent the initial width of the control. The width is given in pixels except when type attribute has the value "text" or "password". In that case, its value refers to the (integer) number of characters. END QUOTE Note that it also depends on font etc. So not a good choice. Bootstrap offers other options for width too. Created attachment 152843 [details] [review] Bug 34146: Add a client-side check with maxlength too Test plan: Try to add more than 3 characters now in the number of copies. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 152844 [details] [review] Bug 34146: Counterpart for serials-edit Not only additem suffers from it. We can do the same with serials-edit. This patch adds a server-side and client-side check as we did for additem. Test plan: Receive serial with adding items. Try to add more than 999 items in number of copies. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 152845 [details] [review] Bug 34146: Do not allow multiple copies to crash server Currently hardcoded to 1000. Can be refined later. Let's first prevent this kind of accidents. Test plan: Add additem.pl. Edit the 1000 to 2. Restart all. Add 3 multiple copies. Notice that you got 2. Revert your code change. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 152846 [details] [review] Bug 34146: Add a client-side check with maxlength too Test plan: Try to add more than 3 characters now in the number of copies. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 152847 [details] [review] Bug 34146: Counterpart for serials-edit Not only additem suffers from it. We can do the same with serials-edit. This patch adds a server-side and client-side check as we did for additem. Test plan: Receive serial with adding items. Try to add more than 999 items in number of copies. Fixed warning on first patch: [2023/06/28 14:14:26] [WARN] CGI::param called in list context from /usr/share/koha/cataloguing/additem.pl line 241, this can lead to vulnerabilities. See the warning in "Fetching the value or values of a single named parameter" at /usr/share/perl5/CGI.pm line 414. Created attachment 152849 [details] [review] Bug 34146: Do not allow multiple copies to crash server Currently hardcoded to 1000. Can be refined later. Let's first prevent this kind of accidents. Test plan: Add additem.pl. Edit the 1000 to 2. Restart all. Add 3 multiple copies. Notice that you got 2. Revert your code change. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 152850 [details] [review] Bug 34146: Add a client-side check with maxlength too Test plan: Try to add more than 3 characters now in the number of copies. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 152851 [details] [review] Bug 34146: Counterpart for serials-edit Not only additem suffers from it. We can do the same with serials-edit. This patch adds a server-side and client-side check as we did for additem. Test plan: Receive serial with adding items. Try to add more than 999 items in number of copies. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Pushed to master for 23.11. Nice work everyone, thanks! Thanks for all the hard work! Pushed to 23.05.x for the next release Nice work everyone! Pushed to oldstable for 22.11.x *** Bug 20932 has been marked as a duplicate of this bug. *** |