Summary: | Check for subscription length value | ||
---|---|---|---|
Product: | Koha | Reporter: | pierre.genty |
Component: | Serials | Assignee: | Baptiste Wojtkowski (bwoj) <baptiste.wojtkowski> |
Status: | Failed QA --- | QA Contact: | Testopia <testopia> |
Severity: | trivial | ||
Priority: | P5 - low | CC: | alban.peyrat_koha, baptiste.wojtkowski, m.de.rooy |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | Trivial patch |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: | ||
Circulation function: | |||
Attachments: |
Bug 36981: Check for subscription length value
Bug 36981: Check for subscription length value |
Description
pierre.genty
2024-05-29 08:57:02 UTC
Created attachment 168308 [details] [review] Bug 36981: Check for subscription length value Original comment: When editing serial subscription, it is possible to set amount such as 999999999 in "Subscription length" field. If no end date is set this could cause an infinite loop. I changed all concerned fields toward number fields with minimum 0 and max 2000 (arbitrary but I think it is nearly impossible to reach such an high number) TEST PLAN: 1 - Apply patch 2 - go to Serials -> search -> pick a subsciption 3 - Click on Edit->Edit subscription 4 - fill correct infos and go to page 2 5 - notice that the second field after the label subscription length is a number field 6 - try and fill -1 in this field -> notice it is impossible to save 7 - try and fill 5000 in this field -> notice it is impossible to save 8 - Repeat the process py clicking on "Renew" at step (3) 9 - Repeat the process py clicking on "numbering patterns" at step (3) (note, you can test the pattern but when clicking done you will be prevented from) Note : changing from enhancement to trivial since providing a too high value may impair the normal operation of Koha Created attachment 168773 [details] [review] Bug 36981: Check for subscription length value Original comment: When editing serial subscription, it is possible to set amount such as 999999999 in "Subscription length" field. If no end date is set this could cause an infinite loop. I changed all concerned fields toward number fields with minimum 0 and max 2000 (arbitrary but I think it is nearly impossible to reach such an high number) TEST PLAN: 1 - Apply patch 2 - go to Serials -> search -> pick a subsciption 3 - Click on Edit->Edit subscription 4 - fill correct infos and go to page 2 5 - notice that the second field after the label subscription length is a number field 6 - try and fill -1 in this field -> notice it is impossible to save 7 - try and fill 5000 in this field -> notice it is impossible to save 8 - Repeat the process py clicking on "Renew" at step (3) 9 - Repeat the process py clicking on "numbering patterns" at step (3) (note, you can test the pattern but when clicking done you will be prevented from) Signed-off-by: David Nind <david@davidnind.com> I am not sure 200 is a good default here. If you pick "issues" and you have a daily newspaper, it would be 365 at least. Saying: I don't think 200 is big enough for common use cases here. Hi Karin, thank you for your reply ! You might have misread the patch, I proposed 2000 as a limit, not 200, is it still too low ? Ooops! (In reply to Baptiste Wojtkowski (bwoj) from comment #5) > Hi Karin, thank you for your reply ! > > You might have misread the patch, I proposed 2000 as a limit, not 200, is it > still too low ? Value is high, but not sure if that is the best approach. Leaving it for proper QA. Thanks for the quick feedback! Understand concerns of Katrin. This does not feel like the best solution. More a workaround. Where does it cause the infinite loop exactly? Can we fix that? Please provide some more details. Please respond to raised questions. Changing status Sry for taking long time to answer, needed to reproduce the pb I did not experiences myself ^^
When loading subscription-details.pl, it computes every occurrence of each record, so if the number of records is too high, it will compute every single occurrence, which may take time. It's not an infinite loop but it can be very long.
I think this comes from here:
> 68
> 69 my ($totalissues,@serialslist) = GetSerials($subscriptionid);
> 70 $totalissues-- if $totalissues; # the -1 is to have 0 if this is a new > subscription (only 1 issue)
> 71
FAIL koha-tmpl/intranet-tmpl/prog/en/modules/serials/subscription-numberpatterns.tt FAIL forbidden patterns forbidden pattern: Don't use type=number for inputs (bug 17098) (line 216) FAIL koha-tmpl/intranet-tmpl/prog/en/modules/serials/subscription-renew.tt FAIL forbidden patterns forbidden pattern: Don't use type=number for inputs (bug 17098) (line 90) |