Skip to content

[Console] Escape exception message during the rendering of an exception #8853

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

Closed
wants to merge 6 commits into from

Conversation

lyrixx
Copy link
Member

@lyrixx lyrixx commented Aug 25, 2013

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets n/a
License MIT
Doc PR n/a

When the exception message contains some HTML, this display is
broken.

I don't know enought about the differents Output classes.
Maybe we have to check the current Output class before
escaping. But for the moment, we allways format the message
with error tags, so I guess the escape is not an issue.

As this is a bug fix, should I re-open this PR against 2.2 branch ?

@lyrixx
Copy link
Member Author

lyrixx commented Aug 25, 2013

For the reccord, here is the output before my patch: (see [0m<p>)



�[37;41m                    �[0m
�[37;41m  [Exception]       �[0m
�[37;41m  Second exception  �[0m
�[37;41m                    �[0m




�[37;41m                                       �[0m
�[37;41m  [Exception]                          �[0m
�[37;41m  First exception �[0m<p>this is html</p>  
�[37;41m                                       �[0m


�[32mfoo3:bar�[0m


@fabpot
Copy link
Member

fabpot commented Aug 26, 2013

IIRC, we do have error messages that have some ANSI sequences.

@lyrixx
Copy link
Member Author

lyrixx commented Aug 26, 2013

ag 'Exception(.*<.*>.*)' src/ returns some matches:

src/Symfony/Component/Routing/Loader/XmlFileLoader.php
src/Symfony/Component/Validator/PropertyMetadataContainerInterface.php
src/Symfony/Component/Console/Tests/Fixtures/Foo3Command.php
src/Symfony/Component/Console/Application.php
src/Symfony/Bundle/TwigBundle/Resources/config/twig.xml
src/Symfony/Bundle/TwigBundle/Resources/views/Exception/exception.html.twig
src/Symfony/Bundle/WebProfilerBundle/Resources/config/profiler.xml
src/Symfony/Bundle/WebProfilerBundle/Resources/views/Collector/exception.html.twig
src/Symfony/Bundle/WebProfilerBundle/Resources/views/Collector/logger.html.twig
src/Symfony/Bundle/FrameworkBundle/Resources/config/collectors.xml
src/Symfony/Bundle/SecurityBundle/Resources/config/security_listeners.xml

The only one which can be "interesting" is src/Symfony/Component/Console/Application.php

if (OutputInterface::VERBOSITY_VERBOSE <= $output->getVerbosity()) {
    $output->writeln('<comment>Exception trace:</comment>');
    // exception related properties
    $trace = $e->getTrace();

But it is not ;)

@fabpot
Copy link
Member

fabpot commented Sep 11, 2013

So, what's the status of this PR?

@lyrixx
Copy link
Member Author

lyrixx commented Sep 11, 2013

IMHO, it's merge-able.

@fabpot
Copy link
Member

fabpot commented Sep 11, 2013

ok, can you do the same PR on 2.2 and add a test when using a valid tag to prove that it does not mess up those?

@lyrixx
Copy link
Member Author

lyrixx commented Sep 11, 2013

What is a valid tag ? something like <comment> foo bar </comment> ?

@fabpot
Copy link
Member

fabpot commented Sep 11, 2013

Everything that is registered as a style on OutputFormatter. I think the replacement code already does that. See the replaceStyle() method there.

jakzal and others added 6 commits September 13, 2013 13:11
This PR was merged into the 2.2 branch.

Discussion
----------

[Translation] Removed a @return annotation

This is the only `@return null` left...

Commits
-------

d6f4def [Translation] Removed an unneeded return annotation.
This PR was merged into the 2.2 branch.

Discussion
----------

[DomCrawler] Added missing docblocks and removed redundant type in a return annotation

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

Commits
-------

d414213 [DomCrawler] Added missing docblocks and removed unneeded return annotation.
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.

3 participants