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

[7.x] FindOrFail being passed null as value on string key returns a model with empty string as key #34029

Closed
RikSomers opened this issue Aug 27, 2020 · 3 comments · Fixed by #34031
Labels

Comments

@RikSomers
Copy link

RikSomers commented Aug 27, 2020

  • Laravel Version: 7.26.0
  • PHP Version: 7.3.3
  • Database Driver & Version: SQL Server 2017

Description:

When you try to fetch a model with a string key with findOrFail, if you pass the value null, it can find a row where the key is an empty string, where previously it would fail to find the model.

Steps To Reproduce:

Given that you have a model with a string primary key:

 public $incrementing = false;

 protected $keyType = 'string';

And given that you have a row in the database with the key being " " (so an empty string).

When you try to fetch the model by passing null to findOrFail

Model::findOrFail(null)

You will receive the row in the database with the empty string.

This has been broken since #33930, as it has been working before on laravel 7.25.0 when this method hadn't been changed.

Our Case

We use a database (not in our control how the database is structured) that has default "empty" keys so it can create cyclic relationships. So for example, a user needs to be created by a user, but for that you need there to be a default user that's created by itself. So this database gets created with foreign key checks disabled, and then afterwards they enable them, because there is now a default "empty" user.

Currently, what we are doing in several places in our code is fetching something based on a value passed in a form request. So what our code currently is doing, is basically the following:

$documentGrp = DocumentGrp::findOrFail(request('doc-grp-code'));

// some other code

$documentGrp->documents()->map(function ($document) {
   // do something with the documents
});

What happened previously was that the findOrFail would fail, because it tried to fetch a model where the key was the literal null value. Since #33930 however, it will typecast the null to a string, so then in turn it will try to find a model that has an empty string as its key.

So instead of executing

Model::findOrFail(null)

In essence, you would be executing the following:

Model::findOrFail('');

Which to me is a completely different findOrFail being executed.

@RikSomers RikSomers changed the title [7.x] FindOrFail on string key returns a model with empty string as key when being passed null [7.x] FindOrFail being passed null as value on string key returns a model with empty string as key Aug 27, 2020
@driesvints
Copy link
Member

Thanks for the detailed report. I'll look into this.

@driesvints
Copy link
Member

Sent in a fix for this: #34031

@driesvints driesvints linked a pull request Aug 27, 2020 that will close this issue
@RikSomers
Copy link
Author

Thanks for the quick response!

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

Successfully merging a pull request may close this issue.

2 participants