Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unable to update nested JSON columns using "->" sign #34630

Closed
yihyang opened this issue Oct 2, 2020 · 6 comments
Closed

Unable to update nested JSON columns using "->" sign #34630

yihyang opened this issue Oct 2, 2020 · 6 comments

Comments

@yihyang
Copy link

yihyang commented Oct 2, 2020

  • Laravel Version: 7.25.0
  • PHP Version: 7.4.6
  • Database Driver & Version: MySQL 5.7.22

Description:

Recently there was a change to verify that columns that are being updated are actual columns:
#33777

Inside this commit, the feature to get the actual column name from nested attribute has been removed
4a15c31#diff-17e37c1b4a410ed3d77fd43a28c094e2

Before the changes, when we specify an update to a JSON attribute, it will get the column name and compare it with the guarded key. For example, when we specify and update as follows:

User::find(123)->update(['wallet->balance' => 100]);

It will use "wallet" to compare against the guarded attributes and therefore will allow the updates

After the changes, "wallet->balance" will be evaluated, returning false inside the isGuardableColumn($key) method and return the isGuarded($key) as true due to the following:

    public function isGuarded($key)
    {
        if (empty($this->getGuarded())) {
            return false;
        }

        return $this->getGuarded() == ['*'] ||
               ! empty(preg_grep('/^'.preg_quote($key).'$/i', $this->getGuarded())) ||
               ! $this->isGuardableColumn($key);
    }

Steps To Reproduce:

  1. Change the version from 7.18.0 to 7.25.0
  2. Perform update to JSON columns using the arrow method.

Related PRs & commits

@taylorotwell
Copy link
Member

This is for security.

@yihyang
Copy link
Author

yihyang commented Oct 2, 2020

@taylorotwell would it work if we allow users to specify instead $fillable using wildcard?

For example, we can have $fillable = ['devices->*'] which will allow users to specify the columns that they wanted to allow the updates.

On the side note, should we consider removing this? https://laravel.com/docs/8.x/queries#updating-json-columns

@taylorotwell
Copy link
Member

That still lets an outside user put ANY key in your JSON column.

@taylorotwell
Copy link
Member

Can't you list "devices->key" explicitly in fillable?

@yihyang
Copy link
Author

yihyang commented Oct 2, 2020

the control of which attributes will be passed in from controllers can be done through request object like $request->only(['devices->key1', 'devices->key2'])

@yihyang
Copy link
Author

yihyang commented Oct 2, 2020

@taylorotwell I'm facing the same issue as mentioned #33975 (comment) as my attributes are dynamic which was why I decided to use JSON instead of creating a column in the first place

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants