Attachment 133062 Details for Bug 30482 – Bug 30482: Potential for bad string concatenation in cataloging validation error message

[patch] Bug 30482: Potential for bad string concatenation in cataloging validation error message

Bug-30482-Potential-for-bad-string-concatenation-i.patch (text/plain), 5.51 KB, created by Owen Leonard on 2022-04-07 09:53:19 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Owen Leonard

Created: 2022-04-07 09:53:19 UTC

Size: 5.51 KB

patch

obsolete

>From 9234b1002bfb915d3068a7f07a832c1bbf8368ec Mon Sep 17 00:00:00 2001
>From: Owen Leonard <oleonard@myacpl.org>
>Date: Thu, 7 Apr 2022 09:36:00 +0000
>Subject: [PATCH] Bug 30482: Potential for bad string concatenation in
> cataloging validation error message
>MIME-Version: 1.0
>Content-Type: text/plain; charset=UTF-8
>Content-Transfer-Encoding: 8bit
>
>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.
>---
> .../prog/en/modules/cataloguing/addbiblio.tt  | 49 ++++++++++---------
> 1 file changed, 26 insertions(+), 23 deletions(-)
>
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/addbiblio.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/addbiblio.tt
>index 4d903160ef..8737d3278d 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/addbiblio.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/addbiblio.tt
>@@ -540,7 +540,7 @@ function PopupMARCFieldDoc(field) {
>         var subfields = new Array();
>         var tab = new Array();
>         var label = new Array();
>-        var flag=0;
>+        var flag = false;
>         var tabflag= new Array();
>         var StrAlert = "<div id='form-errors' class='dialog alert list'>";
>         var notFilledClass = "subfield_not_filled";
>@@ -609,7 +609,7 @@ function PopupMARCFieldDoc(field) {
>                 var tag=tagsubfieldid.substr(0,3);
>                 var subfield=tagsubfieldid.substr(3,1);
>                 StrAlert += "<li>"+_("Tag %s subfield %s %s in tab %s").format(tag, subfield, formatFieldName( tabflag[tagsubfieldid][1] ), tabflag[tagsubfieldid][2]) + ' <a class="linkfield btn btn-link" href="#" data-tab="' + tabflag[tagsubfieldid][2] + '" data-field="' + tabflag[tagsubfieldid][3] + '"><i class="fa fa-arrow-right" aria-hidden="true"></i> ' + _("Go to field") + '</a></li>';
>-                flag=1;
>+                flag = true;
>             }
>         }
>         StrAlert += "</ul>";
>@@ -663,7 +663,7 @@ function PopupMARCFieldDoc(field) {
>             }
> 
>             if(isempty){
>-                flag = 1;
>+                flag = true;
>                 if (mandatory) {
>                     mandatoryFields[ arr[0] ] = {
>                         importance: "mandatory",
>@@ -694,7 +694,7 @@ function PopupMARCFieldDoc(field) {
>             StrAlert += "</ul>";
>         }
>         StrAlert += "</div>";
>-        if (flag) {
>+        if ( flag ) {
>             $("#show-errors").html('<button type="button" class="btn btn-danger show-errors"><i class="fa fa-warning"></i> ' + _("Errors") + '</span>');
>             return StrAlert;
>         } else {
>@@ -703,33 +703,36 @@ function PopupMARCFieldDoc(field) {
>     }
> 
>     /**
>-     *
>-     *
>+     * Run checks for mandatory and important fields
>+     * Output errors if necessary, or submit the form
>      */
>     function Check(){
>         var StrAlert = AreFieldsNotOk();
>-        if( ! StrAlert ){
>-            var StrWarning = AreFieldsNotOk(false);
>-            if (StrWarning){
>-
>-                // Check important fields
>-                $("#check_errors").html( AreFieldsNotOk(false) );
>-                $('html, body').animate({ scrollTop: 0 }, 'fast');
>+        var StrWarning = AreFieldsNotOk( false );
>+        if( !StrAlert && StrWarning ){
>+            // Check important fields
>+            $("#check_errors").html( StrWarning );
>+            $('html, body').animate({ scrollTop: 0 }, 'fast');
> 
>-                var r=confirm(_("Important fields(s) are not filled. Are you sure you want to save?"));
>-                if (! r){
>-                    return false;
>-                }
>+            var r=confirm( _("Important fields(s) are not filled. Are you sure you want to save?" ) );
>+            if (! r){
>+                return false;
>+            } else {
>+                document.f.submit();
>+                return true;
>             }
>-            document.f.submit();
>-            return true;
>-        } else {
>-
>-            // Call AreFieldsNotOk() twice to check both mandatory and important fields
>-            $("#check_errors").html( AreFieldsNotOk() + AreFieldsNotOk(false) );
>+        } else if( StrAlert ){
>+            var strAll = StrAlert;
>+            if( StrWarning ){
>+                strAll += StrWarning;
>+            }
>+            $("#check_errors").html( strAll );
>             $('html, body').animate({ scrollTop: 0 }, 'fast');
>             Sticky.hcSticky('refresh');
>             return false;
>+        } else if( !StrAlert && !StrWarning ){
>+            document.f.submit();
>+            return true;
>         }
>     }
> 
>-- 
>2.20.1

Actions: View | Diff | Splinter Review

Attachments on bug 30482: 133061 | 133062 | 133506 | 133675