Skip to content

Make it clear that the profiler is for dev only #28572

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Sep 24, 2018
Merged

Make it clear that the profiler is for dev only #28572

merged 1 commit into from
Sep 24, 2018

Conversation

fabpot
Copy link
Member

@fabpot fabpot commented Sep 24, 2018

Q A
Branch? master
Bug fix? kinda yes
New feature? yes
BC breaks? no
Deprecations? yes (permanent one)
Tests pass? yes
Fixed tickets n/a
License MIT
Doc PR symfony/symfony-docs#10386

@fabpot fabpot mentioned this pull request Sep 24, 2018
Merged
ro0NL
ro0NL reviewed Sep 24, 2018
View reviewed changes
src/Symfony/Bundle/WebProfilerBundle/WebProfilerBundle.php Outdated
public function boot()
{
if ('prod' === $this->container->getParameter('kernel.environment')) {
@trigger_error('Using WebProfilerBundle in production is not supported and put your project at risk, disable it.', E_USER_DEPRECATED);
Copy link
Contributor

Choose a reason for hiding this comment

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

E_USER_WARNING?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@nicolas-grekas
Copy link
Member

2.8?

@fabpot
Copy link
Member Author

fabpot commented Sep 24, 2018

2.8 is not possible as we had several storage implementations back then.
Could be in 3.4 but adding a warning in a patch version does not look good to me.

@nicolas-grekas
Copy link
Member

The message is still correct on 2.8 to me. Since this involves security, we can do it :)

nicolas-grekas
nicolas-grekas approved these changes Sep 24, 2018
View reviewed changes
src/Symfony/Bundle/WebProfilerBundle/WebProfilerBundle.php Outdated
public function boot()
{
if ('prod' === $this->container->getParameter('kernel.environment')) {
@trigger_error('Using WebProfilerBundle in production is not supported and put your project at risk, disable it.', E_USER_WARNING);
Copy link
Member

Choose a reason for hiding this comment

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

puts?

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

src/Symfony/Component/HttpKernel/Profiler/ProfilerStorageInterface.php Outdated
* As the profiler must only be used in non-production environments, the file storage
* is more than enough and no other implementations will ever be supported.
*
* @internal
Copy link
Member

Choose a reason for hiding this comment

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

since Symfony 4.2?

Copy link
Member Author

Choose a reason for hiding this comment

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

done

@nicolas-grekas nicolas-grekas added this to the next milestone Sep 24, 2018
linaori
linaori reviewed Sep 24, 2018
View reviewed changes
src/Symfony/Bundle/WebProfilerBundle/README.md Outdated
The Web profiler bundle is a **development tool** that gives detailed
information about the execution of any request.

**Never** enable it in production environments as it will lead to major security
Copy link
Contributor

Choose a reason for hiding this comment

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

"Production environment" is ambiguous. Is it the Symfony production environment (prod), or does it represent the production environment of your server?

In case of the latter, it should only be made accessible if really needed, in dev and well secured.

Copy link
Member Author

Choose a reason for hiding this comment

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

Here, we are talking about production servers. I will update the comment to make it clear.

Copy link
Member Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor

Choose a reason for hiding this comment

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

That's better! 👍

nicolas-grekas
nicolas-grekas reviewed Sep 24, 2018
View reviewed changes
src/Symfony/Bundle/WebProfilerBundle/README.md
@@ -1,6 +1,12 @@
WebProfilerBundle
=================

The Web profiler bundle is a **development tool** that gives detailed
Copy link
Member

Choose a reason for hiding this comment

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

should we add this message on branch 2.8 also?

Copy link
Member Author

Choose a reason for hiding this comment

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

see #28575

@fabpot fabpot merged commit 54fda55 into symfony:master Sep 24, 2018
fabpot added a commit that referenced this pull request Sep 24, 2018
@fabpot
feature #28572 Make it clear that the profiler is for dev only (fabpot)
3d2124e 
This PR was merged into the 4.2-dev branch.

Discussion
----------

Make it clear that the profiler is for dev only

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | kinda yes
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | yes (permanent one)
| Tests pass?   | yes
| Fixed tickets | n/a
| License       | MIT
| Doc PR        | symfony/symfony-docs#10386

Commits
-------

54fda55 made it clear that the profiler is for dev only
@nicolas-grekas nicolas-grekas modified the milestones: next, 4.2 Nov 1, 2018
This was referenced Nov 3, 2018
Closed
Merged
@fabpot fabpot deleted the profiler-interface branch January 14, 2019 11:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicolas-grekas nicolas-grekas nicolas-grekas approved these changes

@ro0NL ro0NL ro0NL left review comments

@javiereguiluz javiereguiluz javiereguiluz approved these changes

@linaori linaori linaori approved these changes

@xabbuh xabbuh xabbuh approved these changes

Assignees
No one assigned
Labels
Deprecation Feature Status: Reviewed
Projects
None yet
Milestone
4.2
Development

Successfully merging this pull request may close these issues.
7 participants
@fabpot @nicolas-grekas @javiereguiluz @ro0NL @linaori @xabbuh @carsonbot