Skip to content

[VarDumper] HtmlDumper::setDumpHeader() accepts null #42274

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

Merged
merged 1 commit into from
Jul 28, 2021
Merged

[VarDumper] HtmlDumper::setDumpHeader() accepts null #42274

merged 1 commit into from
Jul 28, 2021

Conversation

rrpadilla
Copy link
Contributor

Q A
Branch? 5.3
Bug fix? yes
New feature? no
Deprecations? no
Tickets Fix #42272
License MIT
Doc PR symfony/symfony-docs#...

@nicolas-grekas
Copy link
Member

Shouldn't maximebf/debugbar be patched instead? Can you please them a PR?

@nicolas-grekas
Copy link
Member

their code does check for NULL headers when printing so they are a possibility.

I read that in the issue on maximebf/debugbar, found the corresponding line in the code. This makes sense as a big fix now.

Can you please add a test case to back the behavior up?

@nicolas-grekas
Copy link
Member

I suggested an alternate fix in php-debugbar/php-debugbar#474

@barryvdh
Copy link
Contributor

I fixed it for the time being now: php-debugbar/php-debugbar#475

I think we needed to reset it for a reason, which isn't currently possible. We're now overriding this method which solves it, but either a nullable check or a reset method would be helpful.

@derrabus
Copy link
Member

I think we needed to reset it for a reason, which isn't currently possible.

Do you want to contribute the reset method as a feature for 5.4?

@nicolas-grekas
Copy link
Member

Actually I prefer making the setter null able as a bugfix :)

@carsonbot carsonbot changed the title HtmlDumper - setDumpHeader accepts null [VarDumper] HtmlDumper - setDumpHeader accepts null Jul 28, 2021
@derrabus derrabus changed the title [VarDumper] HtmlDumper - setDumpHeader accepts null [VarDumper] HtmlDumper::setDumpHeader() accepts null Jul 28, 2021
@derrabus
Copy link
Member

Good catch, thanks @rrpadilla.

@derrabus derrabus merged commit a1055ae into symfony:5.3 Jul 28, 2021
@fabpot fabpot mentioned this pull request Jul 29, 2021
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.

5 participants