https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21052 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org --- Comment #1 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Same for other labels/* forms In the logs: Database returned the following error on attempted INSERT: Duplicate entry 'DEFAULT PRINTER-0-Tray 1-Labels' for key 'printername' at /home/vagrant/kohaclone/C4/Creators/Profile.pm line 164. or profile_id is currently undefined. at /home/vagrant/kohaclone/C4/Creators/Profile.pm line 184. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21052 --- Comment #3 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- *** Bug 22460 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21052 --- Comment #4 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- (In reply to Katrin Fischer from comment #2)

We've also managed to create a template that takes up 100% CPU and brings down Koha when trying to print a PDF. But so far have not been able to figure out how to reproduce it on another system. There are some serious issues with the forms not checking for good input.

We filed bug 22462 for this. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21052 Chris Nighswonger <chris.nighswonger@veritassuperaitsolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=43193 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21052 --- Comment #6 from Chris Nighswonger <chris.nighswonger@veritassuperaitsolutions.com> --- Patch attached. Root cause: printers_profile carries a UNIQUE key on (printer_name, template_id, paper_bin, creator). Neither profile editor validated that the printer name and paper bin were supplied, so a blank field was stored as an empty string. The first blank save succeeded; a second collided on the unique key and the resulting Koha::Exceptions::Object::DuplicateID went unhandled, giving a bare 500 error page. Both editors now trim and validate the two required fields, catch DuplicateID and re-render with a warning, and check save()'s -1 failure return rather than redirecting as though the save had worked. A profile deleted between opening the edit form and saving it also no longer 500s. Rows already left behind by this bug can now be repaired in place -- a blank stored value renders as an editable input rather than read-only, which it previously was. Also worth noting for testers: the new-profile form now arrives pre-filled with 'Default Printer' and 'Tray 1'. Those defaults have been in C4::Creators::Profile->new() all along, but the create branch of both templates emitted an input with no value attribute, so the user saw two empty boxes and an untouched save stored empty strings. The value attribute has to be added anyway so that what the user typed survives an error redisplay; surfacing the defaults is a side effect of that, and is what they were for. Reproduced and verified in koha-testing-docker on both editors, before and after: blank fields, whitespace-only values, duplicate name+bin, cleared numeric fields, repairing a legacy blank row, and a concurrent delete -- no 500s remain in either editor. prove t/db_dependent/Labels/ t/db_dependent/Creators/ t/Creators.t -- 870 tests pass. koha-qa.pl PASS. Two follow-ups are filed rather than folded in here: bug 43191 (cleaning up profiles already saved with a blank value) and bug 43193 (non-numeric offset or creep text, a different defect class). Internal review: https://github.com/cnighswonger/koha-wip/pull/96 An AI tool assisted with the investigation and fix; full detail is in the commit message, and all work was reviewed by me. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21052 Chris Nighswonger <chris.nighswonger@veritassuperaitsolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |aleisha@catalyst.net.nz --- Comment #7 from Chris Nighswonger <chris.nighswonger@veritassuperaitsolutions.com> --- *** Bug 15739 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21052 --- Comment #8 from Chris Nighswonger <chris.nighswonger@veritassuperaitsolutions.com> --- Created attachment 202689 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=202689&action=edit Bug 21052: Validate required fields and handle duplicate printer profiles printers_profile carries a UNIQUE key on (printer_name, template_id, paper_bin, creator). Neither profile editor validated that the printer name and paper bin were supplied, so a blank field was stored as an empty string. The first blank save succeeded; a second one collided on the unique key and the resulting Koha::Exceptions::Object::DuplicateID went unhandled, giving the user a bare 500 error page. The patron card editor passed CGI parameters through untouched. The label editor substituted 'DEFAULT PRINTER' / 'Tray 1' for blanks, which only moved the collision -- two blank saves still produced the same crash there. Validate both fields before saving and re-render the form with a warning when either is empty, catch DuplicateID and re-render with a warning rather than dying, and mark both inputs required in the templates so the browser catches the common case first. Entered values are preserved on redisplay. save() reports some database failures by returning -1 rather than by throwing, so the return value is now checked and routed to an error message instead of a redirect that would imply success. C4::Creators::Profile->new() has always defaulted printer_name to 'Default Printer' and paper_bin to 'Tray 1', but the new-profile form never rendered them: the create branch of both templates emitted an input with no value attribute, so the user was shown two empty boxes and saving an untouched form stored empty strings. Adding the value attribute -- and it has to be added, so that what the user typed survives an error redisplay -- also surfaces those defaults on first load, which is what they were for. A profile deleted between opening the edit form and saving it left retrieve() blessing an undefined value, which died outside the handler as another unhandled 500. That case now redirects back to the profile list with the standard error notice. Because the first blank save succeeded, existing installations have rows with a blank printer name or paper bin. Those fields are stored as hidden inputs on the edit form, which would make such a row unfixable and, worse, silently insert a duplicate. A blank stored value is now rendered as an editable input so the row can be repaired in place, and the two fields stay editable on any error redisplay so a repair that hits a duplicate name can still be corrected. The patron card editor also passed the offset, creep and units fields through without a default, unlike its label twin. Clearing an offset submitted an empty string into a float NOT NULL column; under strict SQL modes that raises an exception which is not DuplicateID and so would be rethrown -- the same unhandled 500 this patch removes. Both editors now apply the same defaults, trim whitespace from the two required fields, and set maxlength to match the column widths. This also fixes bug 15739, which reported the same crash reached by leaving only one of the two fields empty. Test plan: 1) Go to Tools > Patron card creator > Manage > Printer profiles > New profile. 2) Clear 'Printer name' and/or 'Paper bin' and save. (Without this patch both boxes are already empty; with it they arrive pre-filled with the defaults, so they have to be cleared first.) -- without this patch: the profile is silently stored with an empty value; saving a second such profile returns a 500 error page -- with this patch: the form redisplays with "Printer name and paper bin are both required." and nothing is saved 3) Enter only spaces in either field and save. -- the same warning appears; no profile is created 4) Enter a printer name and paper bin, save. Create a second profile with the same name and bin. -- without this patch: 500 error page -- with this patch: the form redisplays with "A printer profile with this printer name and paper bin already exists." 5) Create a profile, then edit it and clear the horizontal offset before saving. -- without this patch, on an installation using strict SQL modes: 500 error page -- with this patch: the offset saves as 0 6) Repeat 1-5 under Cataloging > Label creator > Manage > Printer profiles. Note that on a new profile form both editors now arrive pre-filled with 'Default Printer' and 'Tray 1'; without this patch the two boxes are empty. 7) Simulate a profile left behind by this bug: INSERT INTO printers_profile (printer_name, template_id, paper_bin, offset_horz, offset_vert, creep_horz, creep_vert, units, creator) VALUES ('', 0, '', 0, 0, 0, 0, 'POINT', 'Patroncards'); Edit that profile from the patron card creator. -- without this patch: the blank fields are read-only, saving trips the required-field check, and the form comes back as "New printer profile" -- filling it in inserts a second row and orphans the blank one -- with this patch: the blank fields are editable, the heading still reads "Edit printer profile", and saving repairs the existing row in place 8) Repeat step 7, but give the blank profile a printer name and paper bin that match an existing profile. -- the duplicate warning appears and both fields remain editable, so another name can be entered without leaving the page 9) Open a profile for editing, delete that same profile from the profile list in another browser tab, then save the still-open form. -- without this patch: 500 error page -- with this patch: the profile list is shown with an error notice 10) Edit an existing profile, change an offset, save. -- the profile updates as before 11) prove t/db_dependent/Labels/ t/db_dependent/Creators/ t/Creators.t AI Assistance: Claude Opus 5 (Anthropic) reproduced both bug reports in koha-testing-docker, identified the unique-key collision as the shared root cause (neither report described it correctly), implemented the validation and exception handling, and verified every save scenario across both editors before and after the change. Claude Fable 5 (Anthropic) reviewed two revisions. The first review found the unchecked -1 return from save(), the missing defaults on the patron card editor's numeric fields, and the unrepairable legacy blank rows; the second confirmed those fixes and found the unhandled deleted-profile case and a dead end where a duplicate during an in-place repair left the offending field read-only. OpenAI Codex GPT-5.5 (OpenAI) reviewed the result at the merge gate and found that the deleted-profile redirect interpolated an unvalidated profile_id into a query string, which is now built with URI->query_form. All of those findings are addressed here. The human author directed the investigation, decided to treat bug 15739 as a duplicate rather than patch it separately, and reviewed the change. Assisted-by: Claude Opus 5 (Anthropic) Assisted-by: Claude Fable 5 (Anthropic) Assisted-by: OpenAI Codex GPT-5.5 (OpenAI) -- You are receiving this mail because: You are watching all bug changes.