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

Switching "symfony/var-dumper" in require-dev #2448

Open
simonoche opened this issue May 19, 2022 · 3 comments
Open

Switching "symfony/var-dumper" in require-dev #2448

simonoche opened this issue May 19, 2022 · 3 comments
Labels
Milestone

Comments

@simonoche
Copy link

simonoche commented May 19, 2022

Feature Request

The package symfony/var-dumper is currently required in the main "require" channel.
From my research, there is no valid reason for it to be loaded outside of a dev environment (for mongodb-odm).

I think it is widely accepted that putting this package in a production environment can be dangereous.

The verbosity of the var-dumper can reveal very compromising information. For example, if a dump() or a dd() is inadvertently left in the code, and pushed to production.

Since this package requires the var-dumper in the main channel, then the var-dumper is always installed & loaded on all projects containing mongodb-odm.

Thank you for your attention, hope this can be addressed.

@malarzm
Copy link
Member

malarzm commented May 19, 2022

From my research, there is no valid reason for it to be loaded outside of a dev environment (for mongodb-odm).

The reason for symfony/var-dumper in the require section is this command: https://github.com/doctrine/mongodb-odm/blob/2.4.x/lib/Doctrine/ODM/MongoDB/Tools/Console/Command/QueryCommand.php. I have never used it myself but I reckon it's useful since we're shipping it since the very beginning :)

Theoretically we could move the requirement to require-dev, put it in suggest, and throw an exception in the command if package is not present. But technically that's a BC break. Given dangling var_dump or print_r are as "dangerous" as Symfony's function I'm reluctant to do so right away.

The verbosity of the var-dumper can reveal very compromising information. For example, if a dump() or a dd() is inadvertently left in the code, and pushed to production.

In the meantime while we're giving this a second thought, I'd advise to set simple pre-commit git hooks or even employ CI pipeline for such detections. In our project we're using https://github.com/phpro/grumphp with following task:

        git_blacklist:
            keywords:
                - "die("
                - "var_dump("
                - "print_r("
                - "dump("
                - " dd("
                - "exit;"
            triggered_by: ["php"]

@simonoche
Copy link
Author

You're right, var_dump or print_r may be "dangerous" as well.

However, Symfony's var-dumper package is intended (I guess) for debugging (only?), and is quite popular those days.

If you are not too careful, you easily end up with var-dumper loaded in production ("because of" doctrine-mongodb). Eventually, there may be unpleasant surprises then.

I totally understand your point of view, and the fact that this package is required by a Command. This was a suggestion, but I believe it's a relevant one.

Thanks for the tip, I'll take a look at grumphp

@malarzm
Copy link
Member

malarzm commented May 20, 2022

If you are not too careful, you easily end up with var-dumper loaded in production ("because of" doctrine-mongodb). Eventually, there may be unpleasant surprises then.

Or a fatal error due to an unknown function dump :P Either way one should go all the way to prevent this from happening in the first place ;)

@malarzm malarzm added the Idea label May 20, 2022
@malarzm malarzm added this to the 3.0.0 milestone Oct 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants