Crud: [4.1][Bug] Table field is not properly (?) formatting JSON

[bnxdev]

Bug report

What I did

Using the table field, then adding data to a field that contained a quote.

What I expected to happen

The data to be saved or remembered.

What happened

The JSON isn't valid.

What I've already tried to fix it

Problem seems to lie in table.blade.php on line 208 where you're formatting the inputs for JSON, but I'm not sure exactly the solution, or why this is invalid...

This problem exists on your demo site so let me show you there.

Edit any Monster and go to Miscellaneous and add a row to the table.

Do one without any quotes and save - it works:

Then add a new row with a quote:

Now when you save and go back - the data is gone. It's invalid JSON so is presumably filtered somewhere on your model. For me I have a validator for this JSON and it complains.

The other way this can manifest is with form value persistence. E.g. add a row without a quote and refresh the page - it will still be there. And likewise add a row with a quote and it will not be there on refresh.

Instead you get this error in the console:

I honestly don't know why this seems to happen - the resouces online I've looked at seem to suggest you're escaping the quotes correctly!

Cheers all

[bnxdev]

P.S. I'm working around this at the moment in the controllers by using json_decode(stripslashes()) but this won't fix the client side persistence. This bug is probably something really simple but it's oblivious to me right now.

[bnxdev]

Hi all, my initial solution was misguided. It appears only double quotes (") should be escaped in JSON and not single quotes (') and that the only problem here is line 208 in table.blade.php

var value = this.value.replace(/(['"])/g, "\\$1"); // escapes single and double quotes

Should in fact only filter double quotes.

Changing this no longer causes input to fail JSON validation with Laravel (this is what confused me most), it also causes the form persistence to work properly (pressing F5 during edit) so no more console/javscript errors. It also means my usage of stripslashes in PHP was wrong.

However I wonder if using JSON.stringify is a "better" method? It's what I'm using currently and passes all my tests (line 206 >), it works for all things that need to be esacaped.

if(this.value.length > 0) {
    var key = $(this).attr('data-cell-name').replace('item.','');
    itArr.push('"' + key + '":' + JSON.stringify(this.value));
}

Such a simple bug after all that 😅

[tabacitu]

Such a comprehensive bug report, thanks a lot @bnxdev !

I personally prefer JSON.stringify() over regex too. Any time I can remove a regex, I do. If it doesn't affect anything else, that'd be great. And I don't _think_ it would affect anything else, the only possible problem I see... would be if the contents of a table cell would be something like { x: 5 } and the dev would expect it to be exactly the same in the db, but JSON.stringify() would turn it into {"x":5}. I don't think this is a real-world scenario though, so I think it's ok to use JSON.stringify instead.

@pxpm could you please confirm @bnxdev 's bug and solution?
@bnxdev would you like to submit a PR for this too, so that _you_ show up as the contributor? That would also speed up the process.

Cheers! Thanks again.

[pxpm]

Indeed @tabacitu @bnxdev fixes it for good.

Also from my tests { x : 5 } would still { x : 5 }.

Best,
Pedro

[tabacitu]

Great! Then can you please create a PR for this @pxpm ? I see @bnxdev hasn't replied in 21 days so he's probably busy.

[tabacitu]

Or wait a minute. Isn't this the droid we've been looking for? 👀 https://github.com/Laravel-Backpack/CRUD/pull/2866

[tabacitu]

I've just merged #2960 that fixes this. It'll be available with a composer update later today when we tag version 4.1.12.
Thanks a lot @bnxdev for raising the issue and @pxpm & @ibpavlov for fixing it! I'm sorry it took so long to fix and merge.