-
Notifications
You must be signed in to change notification settings - Fork 372
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
Ignore stderr from openssl commands when saving to a variable #1606
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #1606 +/- ##
===========================================
+ Coverage 66.01% 66.02% +0.01%
===========================================
Files 77 77
Lines 11051 11049 -2
Branches 1558 1557 -1
===========================================
Hits 7295 7295
+ Misses 3422 3421 -1
+ Partials 334 333 -1
Continue to review full report at Codecov.
|
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.
Reviewed 3 of 3 files at r2.
Reviewable status: all files reviewed, 7 unresolved discussions (waiting on @larohra, @narrieta, @pgombar, and @vrdmr)
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.
Reviewable status: all files reviewed, 7 unresolved discussions (waiting on @larohra, @narrieta, @pgombar, and @vrdmr)
azurelinuxagent/common/utils/cryptutil.py, line 65 at r2 (raw file):
Previously, larohra wrote…
that's where shlex.split() helps, I think @pgombar already added that
nope, shlex won't help you there :) take for example file_name=="name with spaces"
azurelinuxagent/common/utils/cryptutil.py, line 65 at r2 (raw file):
Ahh makes sense. |
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.
Reviewable status: all files reviewed, 7 unresolved discussions (waiting on @larohra, @narrieta, @pgombar, and @vrdmr)
azurelinuxagent/common/utils/cryptutil.py, line 65 at r2 (raw file):
Previously, larohra wrote…
{0} x509 -in {1} -pubkey -noout
Ahh makes sense.
What if you escape the filename with quotes instead? Like - [{0} x509 -in '{1}' -pubkey -noout]
This would be handled by shlex
sure, but also quote '{0}' and that would work for now... next person changing the code: please think hard if you need to escape anything :)
or just write the argument array and forget about shell parsing
azurelinuxagent/common/utils/cryptutil.py, line 65 at r2 (raw file): Previously, narrieta (Norberto Arrieta) wrote…
Haha agreed, escaping can always be a pain in the butt! |
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.
Added some suggestions
Description
When calling openssl commands, we aren't ignoring stderr and saving the output directly to a variable, which then causes dictionary key exceptions to be thrown, like in issue #1514.
PR information
Quality of Code and Contribution Guidelines
This change is