check a value is_scalar only on the database fields

[fonsekaean]

The new security check scalarValueOnly affects on any of the variables assigned on to a data object regardless its going to be saved on to the database or not.

By adding an extra check on the condition to see whether the field is actually going to be stored in the database it still let you set / overwrite the values on to a data object.

$myDataObject->Image = $myImage; // this sort of this is denied with the is scalar function when it is overridden. 

Thanks for contributing, you're awesome! ⭐
Please describe expected and observed behaviour, and what you're fixing.
For visual fixes, please include tested browsers and screenshots.
Search for related existing issues and link to them if possible.
Please read https://docs.silverstripe.org/en/contributing/code/

[robbieaverill]

In your example code is your Image property an Image model? I think some more information about why this is causing you issues would be useful, if you don't mind.

[fonsekaean]

Hello @robbieaverill

Image is not a property, but an instance variable I was using to set on the dataobject. So what it does is if I do two assigns then it throws an error, because it tries to check whether the scalarValueOnly on the previously assigned variable.

So by checking the current variable is only a field within the DB then it doesnt check the scalarValueOnly function.

Does it make it clearer ?

Cheers

[kinglozzer]

@fonsekaean Is this correct?

1. You call $this->Image = $obj, it runs fine because $this->dbObject('Image') returns null
2. You call $this->Image = $obj a second time, but it fails this time because $this->dbObject('Image') returns the object that the first call stored

[fonsekaean]

@kinglozzer Yup thats excatly the issue

[kinglozzer]

Could you add a simple unit test to cover this?

[kinglozzer]

Can we reduce the line length here? I believe 120 is our hard-limit

Suggested change

	if ($this->db($fieldName) && ($dbField = $this->dbObject($fieldName)) && $dbField->scalarValueOnly() && !empty($val) && !is_scalar($val)){
	$dbField = ($this->db($fieldName)) ? $this->dbObject($fieldName) : null;
	if ($dbField && $dbField->scalarValueOnly() && !empty($val) && !is_scalar($val)){

[emteknetnz]

This pull request hasn't had any activity for a while. Are you going to be doing further work on it, or would you prefer to close it now?

[emteknetnz]

It seems like there's going to be no further activity on this pull request, so we've closed it for now. Please open a new pull-request if you want to re-approach this work, or for anything else you think could be fixed or improved.