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

[5.3] Prevent calling Model.php methods when calling them as attributes #15438

Merged
merged 3 commits into from
Sep 16, 2016
Merged

[5.3] Prevent calling Model.php methods when calling them as attributes #15438

merged 3 commits into from
Sep 16, 2016

Conversation

themsaid
Copy link
Member

When $user->delete is called, Eloquent will try to get an attribute with name delete, if not found it'll assume it's a name of a relation and try to call the method delete which will throw an exception Relationship method must return an object of type ... however the delete() method will be executed.

This PR checks if the attribute has the name of a method existing in Model.php it returns null and doesn't assume it's a relationship.

@taylorotwell
Copy link
Member

How would you access any relationships if this change is implemented?

@themsaid
Copy link
Member Author

@taylorotwell method_exists(self::class, $key) will check for the existence of the method in the parent Model class only.

I've added more tests to make sure the case for calling a relation is covered.

@taylorotwell
Copy link
Member

Would it still work if you were defining a relation on a model you are extending?

@themsaid
Copy link
Member Author

Yes, having method_exists(self::class, $key) in Model.php means that the check will only look for methods in that specific abstract class Illuminate\Database\Eloquent\Model.

@taylorotwell
Copy link
Member

I guess I'm confused again. If it only looks in the abstract class (Model), how would it find relations defined on my own model (App\User)?

@taylorotwell taylorotwell merged commit 7ce97be into laravel:5.3 Sep 16, 2016
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

Successfully merging this pull request may close these issues.

2 participants