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.8] Fix strict comparison in redis configuration Parsing #28830

Merged
merged 1 commit into from
Jun 13, 2019

Conversation

mathieutu
Copy link
Contributor

Proper fix for issue #28829.

Problem:

  • Input of the function:
    [0 => 'url1', 1 => 'url2', 2 => 'url3']

  • Expected output:
    [0 => 'url1', 1 => 'url2', 2 => 'url3']

  • Actual output:
    [1 => 'url2', 2 => 'url3']

This is about strict comparison in in_array method:

>>> in_array(0, ['foo'])
=> true
>>> in_array(0, ['foo'], true)
=> false

Thanks.
Matt'

PS: Again, this would not happen with tests.

I have no time to add proper tests on all of this RedisManager, but I think this is something we should have. Can be a good first PR for someone who want to contribute.

@driesvints
Copy link
Member

@mathieutu I still feel we should add tests for this, otherwise we risk that this breaks again in a future update. Maybe we should leave this open for a bit until someone can PR in a test to your PR?

@mathieutu
Copy link
Contributor Author

@driesvints Actually this particular PR fixes a bug people have, so we have to merge it as soon as possible, and this is on a very specific part that will not cause any breaking change (just adding strictness on internal check).

@tomswinkels
Copy link
Contributor

@driesvints We can't update Laravel on this moment anymore, this bug are highprio for us. With this fix we are can update Laravel to te newest version.

@taylorotwell taylorotwell merged commit cd4fba8 into laravel:5.8 Jun 13, 2019
@TBlindaruk
Copy link
Contributor

@mathieutu created an issue for the tests #28852

@GrahamCampbell GrahamCampbell changed the title [5.8] Fix strict comparison in redis configuration Parsing. [5.8] Fix strict comparison in redis configuration Parsing Aug 7, 2019
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.

5 participants