| Summary: | Potential for bad string concatenation in cataloging validation error message | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Owen Leonard <oleonard> |
| Component: | Cataloging | Assignee: | Owen Leonard <oleonard> |
| Status: | CLOSED FIXED | QA Contact: | Testopia <testopia> |
| Severity: | trivial | ||
| Priority: | P5 - low | CC: | jonathan.druart, kyle, m.de.rooy, testopia, victor |
| Version: | Main | Keywords: | rel_21_11_candidate |
| Hardware: | All | ||
| OS: | All | ||
| Change sponsored?: | --- | Patch complexity: | Small patch |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: | Version(s) released in: |
22.05.00,21.11.06
|
|
| Circulation function: | |||
| Bug Depends on: | 30159 | ||
| Bug Blocks: | |||
| Attachments: |
Screenshot of an extra "0" output with a validation error
Bug 30482: Potential for bad string concatenation in cataloging validation error message Bug 30482: Potential for bad string concatenation in cataloging validation error message Bug 30482: Potential for bad string concatenation in cataloging validation error message |
||
|
Description
Owen Leonard
2022-04-07 09:31:25 UTC
Comment hidden (obsolete)
We run AreFieldsNotOk() twice and concatenate the result when showing errors on the screen. If the second run of AreFieldsNotOk() returns false, the string concatenation adds "0" Created attachment 133061 [details]
Screenshot of an extra "0" output with a validation error
Created attachment 133062 [details] [review] Bug 30482: Potential for bad string concatenation in cataloging validation error message In validating the basic MARC editor form before submission, we run AreFieldsNotOk() twice (once with the parameter "false") and concatenate the result for output. This creates the potential for the error string to be appended with "0" if AreFieldsNotOk() returns false. This patch improves the logic around building the error string. To test, apply the patch and make sure one of your MARC frameworks contains at least one mandatory field and at least one important field. - Edit or create a MARC record in the basic MARC editor. - Submit the form in various states of completion: - If a mandatory and an important field are empty, you should see two error messages at the top. "The following mandatory subfields aren't filled" and "The following important subfields aren’t filled." - If a mandatory OR an important field is empty, you should see a single message. - If it's the important field which is empty, a confirmation will ask if you want to save the record anyway. Test that both answers to this confirmation work correctly. - If no mandatory or important fields are empty the form should submit. Created attachment 133506 [details] [review] Bug 30482: Potential for bad string concatenation in cataloging validation error message In validating the basic MARC editor form before submission, we run AreFieldsNotOk() twice (once with the parameter "false") and concatenate the result for output. This creates the potential for the error string to be appended with "0" if AreFieldsNotOk() returns false. This patch improves the logic around building the error string. To test, apply the patch and make sure one of your MARC frameworks contains at least one mandatory field and at least one important field. - Edit or create a MARC record in the basic MARC editor. - Submit the form in various states of completion: - If a mandatory and an important field are empty, you should see two error messages at the top. "The following mandatory subfields aren't filled" and "The following important subfields aren’t filled." - If a mandatory OR an important field is empty, you should see a single message. - If it's the important field which is empty, a confirmation will ask if you want to save the record anyway. Test that both answers to this confirmation work correctly. - If no mandatory or important fields are empty the form should submit. Signed-off-by: David Nind <david@davidnind.com> Created attachment 133675 [details] [review] Bug 30482: Potential for bad string concatenation in cataloging validation error message In validating the basic MARC editor form before submission, we run AreFieldsNotOk() twice (once with the parameter "false") and concatenate the result for output. This creates the potential for the error string to be appended with "0" if AreFieldsNotOk() returns false. This patch improves the logic around building the error string. To test, apply the patch and make sure one of your MARC frameworks contains at least one mandatory field and at least one important field. - Edit or create a MARC record in the basic MARC editor. - Submit the form in various states of completion: - If a mandatory and an important field are empty, you should see two error messages at the top. "The following mandatory subfields aren't filled" and "The following important subfields aren’t filled." - If a mandatory OR an important field is empty, you should see a single message. - If it's the important field which is empty, a confirmation will ask if you want to save the record anyway. Test that both answers to this confirmation work correctly. - If no mandatory or important fields are empty the form should submit. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Pushed to master for 22.05, thanks to everybody involved [U+1F984] Pushed to 21.11.x for 21.11.06 Not backported to oldoldstable (21.05.x). Feel free to ask if it's needed. |