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
Don't use Popen(..., shell=True). #11624
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. I agree that it is unlikely anything could go wrong here, but might as well fix it anyway since the shell=True wasn't even needed.
I'll leave separate the question of why we are using Popen, and not something like check_output(), but that can be for a different day.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need the shell here.
Note: This could nowadays be written using subprocess.run
.
There seem to be a conflict, please backport manually |
so, the conflict is that the v2.2.x code had the string literal wrapped with a |
Because Python 2 didn't like Unicode input. |
but, where's the unicode? we weren't using unicode literals
…On Wed, Jul 11, 2018 at 12:38 AM, Elliott Sales de Andrade < ***@***.***> wrote:
Because Python 2 didn't like Unicode input.
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
<#11624 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AARy-APgSoXM_OGtV7ZnU8LFp_1J5xewks5uFYFVgaJpZM4VKMfX>
.
|
ok, then the str() calls aren't needed anymore, that should be simple to fix
…On Wed, Jul 11, 2018 at 12:59 AM, Elliott Sales de Andrade < ***@***.***> wrote:
We were when the str was added: df27722
<df27722>
; unicode literals was removed in fa92b90
<fa92b90>
.
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
<#11624 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AARy-FuIUI4tFoWF1SO9CCzH5gr6OPJuks5uFYYUgaJpZM4VKMfX>
.
|
Don't use Popen(..., shell=True).
PR Summary
This is something that bandit complains about with "high" severity, though I don't think it's actually all that severe in this case.
PR Checklist