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

[HttpFoundation] Fixed type mismatch #42289

Merged
merged 1 commit into from
Jul 27, 2021
Merged

[HttpFoundation] Fixed type mismatch #42289

merged 1 commit into from
Jul 27, 2021

Conversation

Toflar
Copy link
Contributor

@Toflar Toflar commented Jul 27, 2021

Q A
Branch? 4.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets #42290
License MIT
Doc PR -

Fixes

Argument 1 passed to str_contains() must be of the type string, null given, called in /.../vendor/symfony/http-foundation/Response.php on line 327

in case there's no cache-control response header. This doesn't happen by default as the Response is initialized with one by default but any class extending from it can adjust that. Technically speaking, it's not disallowed to have no cache-control header set.

Want me to add a test for that? :)

@carsonbot carsonbot changed the title Fixed type mismatch [HttpFoundation] Fixed type mismatch Jul 27, 2021
@derrabus
Copy link
Member

Want me to add a test for that?

Sure, why not. 🙂

@Toflar
Copy link
Contributor Author

Toflar commented Jul 27, 2021

That was kind of a stupid question, I was just being lazy 🙈 Test is here now :)

@derrabus
Copy link
Member

Thank you @Toflar.

@derrabus derrabus merged commit b68cefa into symfony:4.4 Jul 27, 2021
@Toflar Toflar deleted the patch-1 branch July 27, 2021 14:33
@metaer
Copy link
Contributor

metaer commented Jul 27, 2021

What about 5.3 branch? Is this patch going to be applied to 5.3?

@derrabus
Copy link
Member

Yes.

@bobvandevijver
Copy link
Contributor

I guess this can be reverted, as it was actually a bug with the str_contains polyfill, which just released a new version containing a fix: https://github.com/symfony/polyfill/releases/tag/v1.23.1

This was referenced Jul 29, 2021
@derrabus
Copy link
Member

@bobvandevijver Actually, we need this change again for PHP 8.1 as passing null will trigger a deprecation warning there. I think, we're good.

@bobvandevijver
Copy link
Contributor

@derrabus Ok, but in that case #42300 should probably still be fixed right?

@derrabus
Copy link
Member

Yes. Up for a PR? 🙂

@bobvandevijver
Copy link
Contributor

Done! See #42316 & #42317 😃

@COil
Copy link
Contributor

COil commented Aug 6, 2021

Thanks for the fix. I also had this error with Sf 5.3.5. 😉

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

Successfully merging this pull request may close these issues.

7 participants