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

fix: KINT d()/trace() visual error when activating CSP #5482

Closed
wants to merge 4 commits into from

Conversation

kenjis
Copy link
Member

@kenjis kenjis commented Dec 18, 2021

Description
Fixes #5475

  • fix d()
  • fix trace()

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@kenjis kenjis added the bug Verified issues on the current code behavior or pull requests that will fix them label Dec 18, 2021
@sfadschm
Copy link
Contributor

Can we still use other helpers like dd() this way?

@kenjis kenjis marked this pull request as draft December 19, 2021 07:46
@kenjis
Copy link
Member Author

kenjis commented Dec 21, 2021

Can we still use other helpers like dd() this way?

Yes. I confirmed dd() and trace().

But trace() has the same problem with CSP.
dd() works fine, because it exits and CSP header is not sent.

@kenjis kenjis changed the title fix: KINT d() visual error when activating CSP fix: KINT d()/trace() visual error when activating CSP Dec 21, 2021
@kenjis
Copy link
Member Author

kenjis commented Dec 21, 2021

I added fix for trace().

@kenjis kenjis marked this pull request as ready for review December 21, 2021 08:06
If not, trace() would consume large memory
Copy link
Member

@MGatner MGatner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How will this effect CLI output? Will Kint function modifiers like @ and + still work?

@kenjis
Copy link
Member Author

kenjis commented Dec 23, 2021

@MGatner
Without this PR, you will see the output like this:
screenshott 2021-12-23 20 12 23

But with this PR, @ does not work.
How Kint changes the behavior just adding @?

@kenjis kenjis marked this pull request as draft December 24, 2021 07:48
@MGatner
Copy link
Member

MGatner commented Dec 24, 2021

Have no idea. 🤔 Kint has always seemed like magic to me. I assume it is based on some Reflection trace capabilities.

I don't fully understand what the issue is to begin with so I'm not sure if this trade-off is worth it. I use the Kint modifiers in some projects so this would be a breaking change for me, no clue if others are even aware of them.

@sfadschm
Copy link
Contributor

They actually parse back the source file calling the kint method:
https://stackoverflow.com/a/69890023
https://stackoverflow.com/a/69890287

@kenjis
Copy link
Member Author

kenjis commented Dec 24, 2021

@sfadschm Thanks!

It is too magic.
I take down this PR. This is Kint's with CSP issue.
We shouldn't take care of this.

@kenjis
Copy link
Member Author

kenjis commented Dec 25, 2021

@MGatner I sent another PR #5501
It works with Kint function modifiers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Verified issues on the current code behavior or pull requests that will fix them
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: KINT d() visual error in debugging when activating CSP
3 participants