Htmlawed crashes when Editor popup open longer than a few minutes

Htmlawed crashes when Editor popup open longer than a few minutes

rf1234rf1234 Posts: 2,988Questions: 87Answers: 421

When I create a new record using the Editor popup, and I keep the popup open for longer than a couple of minutes before saving, Htmlawed crashes when I try to save.

In Htmlawed.php it is this line that causes the crash. It seems to be passed an array which it apparently doesn't get passed when I save more quickly.

What can I do to fix this. I am using Editor 2.0.7

$t = explode('<', $t);
Fatal error
Uncaught TypeError: explode(): Argument #2 ($string) must be of type string, array given in /var/www/vhosts/lgfinance.eu/public_html/DataTables/Editor-PHP/lib/HtmLawed/Htmlawed.php:189
Stack trace:
#0 /var/www/vhosts/lgfinance.eu/public_html/DataTables/Editor-PHP/lib/HtmLawed/Htmlawed.php(189): explode()
#1 /var/www/vhosts/lgfinance.eu/public_html/DataTables/Editor-PHP/lib/HtmLawed/Htmlawed.php(110): DataTables\HtmLawed\Htmlawed::hl_bal()
#2 /var/www/vhosts/lgfinance.eu/public_html/DataTables/Editor-PHP/lib/HtmLawed/Htmlaw.php(82): DataTables\HtmLawed\Htmlawed::hl()
#3 /var/www/vhosts/lgfinance.eu/public_html/DataTables/Editor-PHP/lib/Editor/Field.php(792): DataTables\HtmLawed\Htmlaw::filter()
#4 /var/www/vhosts/lgfinance.eu/public_html/DataTables/Editor-PHP/lib/Editor/Field.php(681): DataTables\Editor\Field->xssSafety()
#5 /var/www/vhosts/lgfinance.eu/public_html/DataTables/Editor-PHP/lib/Editor.php(1203): DataTables\Editor\Field->val()
#6 /var/www/vhosts/lgfinance.eu/public_html/DataTables/Editor-PHP/lib/Editor.php(1031): DataTables\Editor->_insert()
#7 /var/www/vhosts/lgfinance.eu/public_html/DataTables/Editor-PHP/lib/Editor.php(701): DataTables\Editor->_process()
#8 /var/www/vhosts/lgfinance.eu/public_html/php/datatables/tblSubReg.php(992): DataTables\Editor->process()
#9 /var/www/vhosts/lgfinance.eu/public_html/actions.php(1708): tblSubReg()
#10 {main}
  thrown in /var/www/vhosts/lgfinance.eu/public_html/DataTables/Editor-PHP/lib/HtmLawed/Htmlawed.php
    on line 189

Answers

  • allanallan Posts: 63,489Questions: 1Answers: 10,470 Site admin
    Answer ✓

    I'm not sure why the amount of time that the modal is open for would make any difference to the Ajax request that is being sent?

    It might be worth updating the Htmlawed.php file with the content from here to see if that helps? It has moved on a few versions since the one I included in Editor 2.0.7.

    Allan

  • rf1234rf1234 Posts: 2,988Questions: 87Answers: 421

    Ok, will try that, Allan.

    My preliminary fix is this one :smiley:

    if ( ! is_array($t) ) {
        $t = explode('<', $t);
    }
    
  • rf1234rf1234 Posts: 2,988Questions: 87Answers: 421

    Hi Allan,

    tried the new version and guess what: It crashes immediately, don't need to keep the modal open long, at exactly the same point which this time is in line 391:

    $t = explode('<', $t);
    

    I'll try to figure this out with my debugger and will get back here.

  • rf1234rf1234 Posts: 2,988Questions: 87Answers: 421

    HTMLawed gets passed this array which makes it crash:

    This is the field in the client side Editor instance:

    }, {
        label: lang === 'de' ? 
                'Wählen Sie eine oder mehrere zu verknüpfende Maßnahmen:' : 
                'Please select one ore more subsidies to be linked:',
        name: "ctr_has_ctr[].linked_ctr_id", //render serial, name
        type: "selectize" //must be a selectize field ALWAYS!!
    }, {
    

    The field gets cleared and re-added on Editor "open" because it is not possible to update the options for a selectize field (I wrote about that before):

    $.ajax({
        type: "POST",
        url: 'actions.php?action=getCtrLinkOptions',
        data: {
            ctr_id: action === "create" ? 0 : parentId
        },
        dataType: "json",
        success: function (data) {
            that.clear( "ctr_has_ctr[].linked_ctr_id" );
            that.add( {
                label: lang === 'de' ? 
                            'Wählen Sie eine oder mehrere zu verknüpfende Maßnahmen:' : 
                            'Please select one ore more subsidies to be linked:',
                name: "ctr_has_ctr[].linked_ctr_id", //render serial, name
                type: "selectize",
                options: data,
                opts: {
                        create: false,
                        maxItems: null,
                        openOnFocus: true,
                        allowEmptyOption: true,
                        placeholder: lang === 'de' ? 
                            'Keine verknüpften Maßnahmen vorhanden' : 'No linked subsidies exist'
                        }
            }, "ctr.file_number" );
        }
    });
    

    And this is the relevant part in PHP:

    Field::inst( 'ctr.id AS ctr_has_ctr' )->set( false )   //return same format as an MJoin             
        ->getFormatter( function($val, $data, $opts) use ( $db ){
            $stmt = ('SELECT b.linked_ctr_id, a.serial, c.sub_name 
                        FROM ctr a  
                  INNER JOIN ctr_has_ctr b ON a.id = b.linked_ctr_id 
                  INNER JOIN sub c         ON a.id = c.ctr_id 
                       WHERE b.ctr_id = :ctr_id 
                    ORDER BY 1 ASC');  
            $result = $db ->raw()
                          ->bind(':ctr_id',$val)
                          ->exec($stmt);
            return $result->fetchAll(PDO::FETCH_ASSOC);
        } ),
    

    @allan, what do you think? What causes this issue?

  • rf1234rf1234 Posts: 2,988Questions: 87Answers: 421

    Some more detail ... as you can see above I don't update "ctr_has_ctr" in the Field instance for "ctr_has_ctr". This is how I do it:

    I use the raw() method to delete the existing links and reinsert new ones using Editor's insert() method. I would guess that the insert() method calls HTMLawed with an array instead of a string and hence causes the issue.

    Is that so, Allan? I am only guessing :smile:

    ->on('writeCreate', function ( $editor, $id, $values ) use ( $db ) {
        processCtrLinkedContracts ( $values, $id, $db );
    } )
    ->on( 'writeEdit', function ( $editor, $id, $values ) use ( $db ) {
        processCtrLinkedContracts ( $values, $id, $db );
    } )
    ....
    function processCtrLinkedContracts ( $values, $id, $db ) {
        //if comment or description: do nothing!
        if ( ! isset($values['ctr_has_ctr-many-count']) ) {
            return;
        }
        $db->raw()
           ->bind( ':id', $id )
           ->exec( 'DELETE FROM ctr_has_ctr
                     WHERE ctr_id        = :id
                        OR linked_ctr_id = :id' );
    
        for ($i = 0; $i < $values['ctr_has_ctr-many-count']; $i++) {
            $linkedId = $values["ctr_has_ctr"][$i]["linked_ctr_id"];
            $res = $db->insert( 'ctr_has_ctr', array (
                'ctr_id'        => $id,
                'linked_ctr_id' => $linkedId
            ) );
            $res = $db->insert( 'ctr_has_ctr', array (
                'ctr_id'        => $linkedId,
                'linked_ctr_id' => $id
            ) );
        }
    }
    
  • allanallan Posts: 63,489Questions: 1Answers: 10,470 Site admin
    Answer ✓

    I would guess that the insert() method calls HTMLawed with an array instead of a string and hence causes the issue.

    Actually no, HtmlLawed is only invoked by the Field class.

    Can you show me the HTTP parameters and values that are being submitted please?

    Also, do you have a Field instance that refers to linked_ctr_id in its name?

    Allan

  • rf1234rf1234 Posts: 2,988Questions: 87Answers: 421
    edited February 7

    Also, do you have a Field instance that refers to linked_ctr_id in its name?

    no, there is none. Only what I posted above.

    Can you show me the HTTP parameters and values that are being submitted please?

    I did this with my fix above implemented. So it didn't crash this time.
    The values:

    data[0][sub][sub_name]: Test
    data[0][sub][sub_program]: Test
    data[0][sub][sub_partner_id]: 1
    data[0][sub][sub_short_desc]: <div>Bla Bla  </div>
    data[0][sub][sub_ratio]: 80
    data[0][sub][app_due_date]: 15.02.2024
    data[0][sub][status_comment]: <div>ölajsdfölasfd</div>
    data[0][sub][main_project]: 1
    data[0][ctr][file_number]: bla
    data[0][ctr][links]: https://www.lgfinance.eu, www.lippstadt.de
    data[0][ctr][soft_deleted]: 
    data[0][ctr][id]: 
    data[0][ctr][serial]: 
    data[0][file][0][id]: 72549
    data[0][file-many-count]: 1
    data[0][ctr_label][0][id]: 2480
    data[0][ctr_label][1][id]: 2663
    data[0][ctr_label-many-count]: 2
    data[0][ctr_govdept][0][id]: 2209
    data[0][ctr_govdept][1][id]: 1593
    data[0][ctr_govdept-many-count]: 2
    data[0][ctr_has_ctr][0][linked_ctr_id]: 3767
    data[0][ctr_has_ctr][1][linked_ctr_id]: 3768
    data[0][ctr_has_ctr-many-count]: 2
    action: create
    

    The headers:

    General:

    Request URL:
    http://localhost/lgf/actions.php?action=tblSubReg
    Request Method:
    POST
    Status Code:
    200 OK
    Remote Address:
    [::1]:80
    Referrer Policy:
    strict-origin-when-cross-origin
    

    Request Headers:

    Accept:
    application/json, text/javascript, */*; q=0.01
    Accept-Encoding:
    gzip, deflate, br
    Accept-Language:
    de-DE,de;q=0.9,en-US;q=0.8,en;q=0.7,nl;q=0.6
    Connection:
    keep-alive
    Content-Length:
    1171
    Content-Type:
    application/x-www-form-urlencoded; charset=UTF-8
    Cookie:
    vatDisc=1; _gcl_au=1.1.1188162673.1701699056; cookieconsent_status=dismiss; vatNews4=1; lang=de; subNews4=1; ctrNews5=1; credNews7=1; PHPSESSID=as97kmq22qolnrbp0atl196ivb; subRegServerSide=0; ctrMgmtServerSide=0
    Host:
    localhost
    Origin:
    http://localhost
    Referer:
    http://localhost/lgf/?page=subRegDocSearch
    Sec-Ch-Ua:
    "Not A(Brand";v="99", "Google Chrome";v="121", "Chromium";v="121"
    Sec-Ch-Ua-Mobile:
    
    <?php
    0
    Sec-Ch-Ua-Platform:
    "Windows"
    Sec-Fetch-Dest:
    empty
    Sec-Fetch-Mode:
    cors
    Sec-Fetch-Site:
    same-origin
    User-Agent:
    Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/121.0.0.0 Safari/537.36
    X-Requested-With:
    XMLHttpRequest
    ?>
    
    
    
  • allanallan Posts: 63,489Questions: 1Answers: 10,470 Site admin

    Its a weird one! It has got to be that data[0][ctr_has_ctr] is being passed as a field somewhere and Htmlawed is tripping over that array.

    Could you perhaps email me the full PHP file? allan at this domain.

    Thanks,
    Allan

  • rf1234rf1234 Posts: 2,988Questions: 87Answers: 421

    Will send you the file.

    Just took a look at my debugger again. The $values parameter on "writeEdit" or "writeCreate" looks fine:

    And what arrives from the server looks perfectly fine as well.

    Could this cause the problem? (see the full code above). This field instance returns the above.

    Field::inst( 'ctr.id AS ctr_has_ctr' )->set( false ) 
    
  • allanallan Posts: 63,489Questions: 1Answers: 10,470 Site admin
    Answer ✓

    Many thanks for the file!

    There is something peculiar going on - the backtrace makes it appear that it is this line which triggering the XSS formatting, that results in the failure. But your ctr_has_ctr field has ->set(false). I don't recall any errors in 2.0.x which would result in a non-settable field being set!

    Is your debugger able to say what $field->name() is at that point immediately before the error? (Perhaps even just an echo statement might do).

    I have thought of a workaround (assuming it is that field which is failing) - add ->xss(false) to it to bypass the Htmlawed stuff.

    Thanks,
    Allan

  • allanallan Posts: 63,489Questions: 1Answers: 10,470 Site admin
    Answer ✓

    The data you highlight is what is causing the issue - Editor is not expecting an array for the field value. Even if we resolve why it is being used on line 1203, there are a couple of other places that it might be needed, so that isn't a complete solution.

    I could put a condition on the XSS function to see if it is given an array, but if I did, I'd want to throw an error since it is unexpected input and the result is undefined (currently an error!).

    I'm thinking that ->xss(false) is the correct way to handle this error you are seeing.

    Allan

  • rf1234rf1234 Posts: 2,988Questions: 87Answers: 421
    edited February 8

    Thanks for that Allan! Guess what I found in a different file for a different business solution that contains the exact same field linking contracts to eachother and vice versa. I ran into this issue years ago - and didn't remember it. Sorry.

    I don't really understand my own comment, but anyway :smile:
    Why should this have anything to do with "copy & new"? In the other solution it also crashed when I just did "New", not "copy & new". In fact I don't have "copy & new" in the other solution because it is much too complex. So the copying is done on the server.

    Field::inst( 'ctr.id AS ctr_has_ctr' )->set( false )   //return same format as an MJoin   
        //make sure copy & new also works for this field!!
        //xss expects a string but we send an array!!
        ->xss( false )
        ->getFormatter( function($val, $data, $opts) use ( $db ){
            $stmt = ('SELECT b.linked_ctr_id, a.serial, a.ctr_name
                        FROM ctr a  
                  INNER JOIN ctr_has_ctr b ON a.id = b.linked_ctr_id
                       WHERE b.ctr_id = :ctr_id 
                    ORDER BY 1 ASC');  
            $result = $db ->raw()
                          ->bind(':ctr_id',$val)
                          ->exec($stmt);
            return $result->fetchAll(PDO::FETCH_ASSOC);
        } ),
    

    This is the "copy & new" button I mentioned above:

    //custom button for edit / create = Allan's "duplicate" button = ctrManagement table
    $.fn.dataTable.ext.buttons.duplicate = {
        extend: "selected",
        text: editCreateLabel,
        action: function ( e, dt, node, config ) {
            // Start in edit mode, and then change to create
            editor
                .title($.fn.dataTable.Editor.defaults.i18n.create.title)
                .buttons({
                        label: $.fn.dataTable.Editor.defaults.i18n.create.submit,
                        className: 'btn-showall-color',
                        fn: function () {
                            this.submit();
                        }
                    })
                .edit( dt.rows( {selected: true} ).indexes() )
                .mode( 'create' );
        },
        className: "duplicateButton"
    };
    
  • allanallan Posts: 63,489Questions: 1Answers: 10,470 Site admin
    Answer ✓

    Why should this have anything to do with "copy & new"?

    Possibly different words for "duplicate" and "create" (which is what I tend to use for Editor - but either would work)? The second line of the comment is bob on though!

    Glad to hear you got it working!

    Allan

  • rf1234rf1234 Posts: 2,988Questions: 87Answers: 421

    Hi Allan,

    I still don't understand why the problem occurs here and not in other cases. Should HTMLawed be called at all in this case? I mean the field is "->set( false )"? Why is it called for a field that will not be set?

    Could it be that HTMLawed is only called because I use Editor's db handler for the query? In most cases I use my own and have a function that returns the value. Never had any problem here for example:

    Field::inst( 'ctr_govdept.id AS gov' )->set( false )   //return same format as an MJoin             
        ->getFormatter( function($val, $data, $opts) {
            return getFormatterGovArray($val);
        }),
    ....
    function getFormatterGovArray($ctrGovdeptId) {
        global $dbh;
        
        $dbh->query('SELECT DISTINCT a.name AS govName, a.regional_12 AS govRegional12
                       FROM gov a
                 INNER JOIN ctr_installation_has_gov b          ON a.id = b.gov_id
                 INNER JOIN ctr_installation c                  ON b.ctr_installation_id = c.id
                 INNER JOIN ctr_govdept_has_ctr_installation d  ON c.id = d.ctr_installation_id
                      WHERE d.ctr_govdept_id = :ctrGovdeptId
                   ORDER BY 1 ASC');
        $dbh->bind(':ctrGovdeptId', $ctrGovdeptId); 
        
        return $dbh->resultsetAssoc();     
    }
    

    As you can see here I am using this very often and only twice it causes issues:

    Roland

  • allanallan Posts: 63,489Questions: 1Answers: 10,470 Site admin

    I mean the field is "->set( false )"? Why is it called for a field that will not be set?

    Not for the main set - which is where the backtrace is happening, and I'm not sure why that is happening. I would need to be able to replicate the issue and debug it. I've made a note to look into it when I get a chance.

    However, there are two other parts of the code which it could legitimately attempt to get the value, and that would trigger the XSS routine. So even if I did resolve the other one, then it could still happen under certain circumstances.

    I think the key is that an array should not be passed as a field value, unless the field has a formatter that converts it in some way.

    Allan

  • rf1234rf1234 Posts: 2,988Questions: 87Answers: 421

    I think the key is that an array should not be passed as a field value, unless the field has a formatter that converts it in some way.

    Well, I don't see me passing an array to the server. If I take a look at the server submission above. And I double checked it right now. Something else must pass the array to the XSS routine ... I am not calling it anywhere.

  • allanallan Posts: 63,489Questions: 1Answers: 10,470 Site admin
    data[0][ctr_has_ctr][0][linked_ctr_id]: 3767
    data[0][ctr_has_ctr][1][linked_ctr_id]: 3768
    

    is it not?

    Allan

  • rf1234rf1234 Posts: 2,988Questions: 87Answers: 421
    edited February 9

    I looked at those as individual strings ... Maybe I am wrong.

    But: It is exactly the same as here:

    data[0][ctr_label][0][id]: 2480
    data[0][ctr_label][1][id]: 2663
    data[0][ctr_label-many-count]: 2
    data[0][ctr_govdept][0][id]: 2209
    data[0][ctr_govdept][1][id]: 1593
    data[0][ctr_govdept-many-count]: 2
    

    And those are Editor Mjoins and have never caused any issues

    ->join(
    Mjoin::inst( 'ctr_govdept' )
        ->link( 'ctr.id', 'ctr_has_ctr_govdept.ctr_id' )
        ->link( 'ctr_govdept.id', 'ctr_has_ctr_govdept.ctr_govdept_id' )
        ->order( 'ctr_govdept.dept_name asc' )
        ->fields(
            Field::inst( 'id' )->set( false ),
            Field::inst( 'dept_name' )->set( false ),
            Field::inst( 'is_subsidy_register' )->set( false )   //wichtig: wegen Log Anzeige!! Wird mit geloggt!
        )
    )
    ->join(
    Mjoin::inst( 'ctr_label' )
        ->link( 'ctr.id', 'ctr_has_ctr_label.ctr_id' )
        ->link( 'ctr_label.id', 'ctr_has_ctr_label.ctr_label_id' )
        ->order( 'ctr_label.label_text asc' )
        ->fields(
            Field::inst( 'id' )->set( false ),
            Field::inst( 'label_text' )->set( false )
        )
    )
    

    Maybe you can figure this out some time when you get bored :smile:. Probably never going to happen. The only difference I can tell is that "id" gets passed instead of "linked_ctr_id". Is there an "exception" for "id"?

  • allanallan Posts: 63,489Questions: 1Answers: 10,470 Site admin
    Answer ✓

    That one is okay because it is an Mjoin. Editor is expecting an array there and will handle it correctly.

    The ctr_has_ctr field though is just a top level field, and it isn't expecting an array there - hence the error.

    Allan

  • rf1234rf1234 Posts: 2,988Questions: 87Answers: 421

    Then there is no solution for this except to turn XSS off, I guess. Mjoin isn't flexible enough for me often times. In many cases I have a more complex data structure than an Mjoin could handle.

  • allanallan Posts: 63,489Questions: 1Answers: 10,470 Site admin
    Answer ✓

    Then there is no solution for this except to turn XSS off

    Agreed. The fields just aren't expecting an array - there are a number of things that would need to change for such a case. The closest thing to a "fix" in the short term would be to throw an error message saying that a field doesn't support arrays. If this crops up again I'll look at adding that in.

    Allan

  • rf1234rf1234 Posts: 2,988Questions: 87Answers: 421
    edited February 9

    The closest thing to a "fix" in the short term would be to throw an error message saying that a field doesn't support arrays.

    Ok, Allan but please allow to mitigate this by turning XSS off. Otherwise I'd be having a problem.

    And finally I understand why the situation is different compared with my other "pseudo"-Mjoins: This one does send data back to the server, the others don't: They are only sending data in "Mjoin"-format from the server to the client. Nothing's sent back!

    If you look at my post above that has the "writeCreate" and "writeEdit" in it, you see how I process the "ctr_has_ctr"-values coming from the client. (function processCtrLinkedContracts).

    Learned something today :smile:

  • allanallan Posts: 63,489Questions: 1Answers: 10,470 Site admin
    Answer ✓

    Ok, Allan but please allow to mitigate this by turning XSS off. Otherwise I'd be having a problem.

    Yup, if I ever did throw an error for such a case, it would likely be the same place as where the current unhandled error is. So the ->xss(false) current workaround would continue to work.

    Allan

Sign In or Register to comment.