Welcome Guest, Not a member yet? Register   Sign In
Model::update() is dangerous
#14

kenjis Wrote:But have you guys ever forgotten to validate input values?

Ok, this is hard to argue. This probably happened to everyone.

kenjis Wrote:I can't think of a use case like this sample where you want to update all records at the same time when updating a single record.

As I said before. We have to think of it from the point of view of the entire API that comes with the Model.
Developers may write queries that don't necessarily update all the records when the null is set, like:

PHP Code:
$model->where(...)->set(...)->update(); 

So, if we suddenly start to require an $id, this will break people's code. Not sure what is more damaging.

The only workaround I see for this is checking if the where part was set before or not. Something like:

PHP Code:
    protected function doUpdate($id null$data nullbool $allowNullId false): bool
    
{
        $escape      $this->escape;
        $this->escape = [];

        $builder $this->builder();

        if ($id === null && ! $allowNullId && empty($builder->getCompiledQBWhere())) {
            throw new DatabaseException(
                'Updates are not allowed unless they contain a "where" or "like" clause.'
            );
        }

        if ($id) {
            $builder $builder->whereIn($this->table '.' $this->primaryKey$id);
        }

        // Must use the set() method to ensure to set the correct escape flag
        foreach ($data as $key => $val) {
            $builder->set($key$val$escape[$key] ?? null);
        }

        return $builder->update();
    

It would still be a BC break, but the overall functionality would remain. Still, I'm not sure this is the best thing to do.
Reply


Messages In This Thread
Model::update() is dangerous - by kenjis - 11-15-2022, 12:36 AM
RE: Model::update() is dangerous - by ozornick - 11-15-2022, 01:46 AM
RE: Model::update() is dangerous - by iRedds - 11-15-2022, 06:21 AM
RE: Model::update() is dangerous - by kenjis - 11-15-2022, 04:45 PM
RE: Model::update() is dangerous - by ikesela - 11-15-2022, 07:51 AM
RE: Model::update() is dangerous - by ozornick - 11-15-2022, 08:07 AM
RE: Model::update() is dangerous - by iRedds - 11-15-2022, 09:12 PM
RE: Model::update() is dangerous - by kenjis - 11-19-2022, 04:51 PM
RE: Model::update() is dangerous - by kenjis - 11-15-2022, 10:45 PM
RE: Model::update() is dangerous - by InsiteFX - 11-15-2022, 11:02 PM
RE: Model::update() is dangerous - by kenjis - 11-16-2022, 05:07 AM
RE: Model::update() is dangerous - by ikesela - 11-16-2022, 07:32 AM
RE: Model::update() is dangerous - by michalsn - 11-16-2022, 10:54 AM
RE: Model::update() is dangerous - by kenjis - 11-16-2022, 02:47 PM
RE: Model::update() is dangerous - by michalsn - 11-17-2022, 09:12 AM
RE: Model::update() is dangerous - by kenjis - 11-17-2022, 05:28 PM
RE: Model::update() is dangerous - by kenjis - 11-17-2022, 05:26 PM
RE: Model::update() is dangerous - by michalsn - 11-18-2022, 03:42 AM
RE: Model::update() is dangerous - by InsiteFX - 11-19-2022, 11:42 PM
RE: Model::update() is dangerous - by kenjis - 11-28-2022, 04:52 PM
RE: Model::update() is dangerous - by iRedds - 11-29-2022, 02:07 PM
RE: Model::update() is dangerous - by kenjis - 11-29-2022, 04:53 PM



Theme © iAndrew 2016 - Forum software by © MyBB