Htmlawed crashes when Editor popup open longer than a few minutes

rf1234

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

allan

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

rf1234

Ok, will try that, Allan.

My preliminary fix is this one

if ( ! is_array($t) ) {
    $t = explode('<', $t);
}

rf1234

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.

rf1234

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?

rf1234

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

->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
        ) );
    }
}

allan

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

rf1234

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:
?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

allan

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

rf1234

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 ) 

allan

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

allan

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

rf1234

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
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"
};

allan

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

rf1234

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

allan

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

rf1234

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.

allan

data[0][ctr_has_ctr][0][linked_ctr_id]: 3767
data[0][ctr_has_ctr][1][linked_ctr_id]: 3768

is it not?

Allan

rf1234

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 . 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"?

allan

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

rf1234

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.

allan

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

rf1234

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

allan

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