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

[10.x] fix(Eloquent/Builder): calling the methods on passthru base object should be case-insensitive #48852

Conversation

luka-papez
Copy link
Contributor

fix(Eloquent/Builder): calling the methods on passthru base object should be case-insensitive

Fixes: #48825

The fix is quite straightforward: when deciding if the method called should be proxied to the base Builder object, compare the lowercased name of the method being called instead of the raw name.

There is an alternative proposal for a more robust solution in the linked Github issue, but I only saw the discussion after committing.

@luka-papez luka-papez force-pushed the fix/eloquent-builder-passthru-case-sensitivity branch 3 times, most recently from 7ffaa69 to 986db63 Compare October 29, 2023 11:34
@luka-papez luka-papez force-pushed the fix/eloquent-builder-passthru-case-sensitivity branch from 986db63 to 6290de8 Compare October 29, 2023 11:45
@bertoost
Copy link

bertoost commented Nov 14, 2023

This will break existing $passthru lists which has correct casing. The list should be compared lowercased as well.

I have extended the Query Builder, and configured toSql to passthru, but it suddenly breaks (and returns the Builder instead) because of this casing. Once I lowercased the $passthru list, it worked again.

This should work;

if (in_array(strtolower($method), array_map('strtolower', $this->passthru))) {
    // ...
}

@luka-papez
Copy link
Contributor Author

Hi @bertoost

I didn't get notified about your comment, most likely because the issue is closed. I suspect the maintainers didn't get notified either.

I see the problem, I did wonder if lowercasing the $passthru property would break someone's code and inevitably it did 🙈

The fix you proposed sounds reasonable to me - I think you should go ahead an open a pull request, the maintainers are very responsive and will gladly merge it in. If you need help, feel free to ping me here, I will subscribe to the thread.

@GrahamCampbell GrahamCampbell changed the title fix(Eloquent/Builder): calling the methods on passthru base object should be case-insensitive [10.x] fix(Eloquent/Builder): calling the methods on passthru base object should be case-insensitive Dec 11, 2023
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.

Result of a method call on Eloquent\Builder depends on the method casing used
3 participants