Skip to content

[Console] Escape command usage #24455

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
Oct 8, 2017

Conversation

sroze
Copy link
Contributor

@sroze sroze commented Oct 6, 2017

Q A
Branch? 2.7
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #24225
License MIT
Doc PR ø

Escape the console usage to prevent arguments named info or similar to be formatted.

@ro0NL
Copy link
Contributor

ro0NL commented Oct 6, 2017

What about markdown formatter? should we use htmlspecialchars there?

@sroze
Copy link
Contributor Author

sroze commented Oct 6, 2017

@ro0NL I'm not sure we should change anything on the Markdown usage for now, I don't see any issue with such arguments.

@ro0NL
Copy link
Contributor

ro0NL commented Oct 6, 2017

AFAIK markdown will render it as HTML tags;

<argument>       # shown as HTML; thus not visible
`<argument>`     # shown as code
&lt;argument&gt; # shown as text

If we already quote using backticks we're good :)

edit: https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Console/Tests/Fixtures/command_2.md looks good already, i think we're fine :)

Copy link
Contributor

@ro0NL ro0NL left a comment

Choose a reason for hiding this comment

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

I think we should patch 2.7 instead.

@xabbuh xabbuh added this to the 3.4 milestone Oct 6, 2017
@sroze sroze force-pushed the bug-24225-escape-command-usage branch from 5b562d5 to ef650dd Compare October 7, 2017 11:18
@sroze
Copy link
Contributor Author

sroze commented Oct 7, 2017

ping @nicolas-grekas

@chalasr
Copy link
Member

chalasr commented Oct 8, 2017

I also think this should be merged as a bugfix in 2.7. @sroze Can you retarget and rebase?

@sroze sroze force-pushed the bug-24225-escape-command-usage branch from ef650dd to 2141056 Compare October 8, 2017 12:00
@sroze sroze changed the base branch from 3.4 to 2.7 October 8, 2017 12:00
@sroze
Copy link
Contributor Author

sroze commented Oct 8, 2017

Good point. @chalasr rebased and changed target 👍

@chalasr
Copy link
Member

chalasr commented Oct 8, 2017

Thank you @sroze.

@chalasr chalasr merged commit 2141056 into symfony:2.7 Oct 8, 2017
chalasr pushed a commit that referenced this pull request Oct 8, 2017
This PR was merged into the 2.7 branch.

Discussion
----------

[Console] Escape command usage

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

Escape the console usage to prevent arguments named `info` or similar to be formatted.

Commits
-------

2141056 Escape command usage when displaying it in the text descriptor
@sroze sroze deleted the bug-24225-escape-command-usage branch October 8, 2017 12:20
This was referenced Nov 10, 2017
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.

6 participants