Formed in 2009, the Archive Team (not to be confused with the archive.org Archive-It Team) is a rogue archivist collective dedicated to saving copies of rapidly dying or deleted websites for the sake of history and digital heritage. The group is 100% composed of volunteers and interested parties, and has expanded into a large amount of related projects for saving online and digital history.
Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upFix printf escape errors in shell scripts #3614
Conversation
Showing a regexp to say that by default all tests are executed is not particularly helpful. If we ever add a default exclusion list or a default filter, we can edit the documentation again. Signed-off-by: Gilles Peskine <[email protected]>
Fix `printf "$foo"` which treats the value of `foo` as a printf format
rather than a string.
I used the following command to find potentially problematic lines:
```
git ls-files '*.sh' | xargs egrep 'printf +("?[^"]*|[^ ]*)\$'
```
The remaining ones are false positives for this regexp.
The errors only had minor consequences: the output of `ssl-opt.sh`
contained lines like
```
Renegotiation: gnutls server strict, client-initiated .................. ./tests/ssl-opt.sh: 741: printf: %S: invalid directive
PASS
```
and in case of failure the GnuTLS command containing a substring like
`--priority=NORMAL:%SAFE_RENEGOTIATION` was not included in the log
file. With the current tests, there was no risk of a test failure
going undetected.
Signed-off-by: Gilles Peskine <[email protected]>
|
Just two minor questions. |
| @@ -433,10 +433,11 @@ run_test "Binary file instead of text file" \ | |||
|
|
|||
| # End of tests | |||
|
|
|||
| echo | |||
ronald-cron-arm
Sep 24, 2020
Contributor
Why moving to echo here ? Just wondering if there is some specific reason.
Why moving to echo here ? Just wondering if there is some specific reason.
gilles-peskine-arm
Sep 24, 2020
Author
Contributor
I find echo clearer than printf since you don't have to worry about escape sequences. They each have their downside: echo only works portably on strings that don't contain backslashes and don't start with a dash, but it's straightforward for those strings. printf is well-defined but you need to remember about its escapes (and it's slower in some shells where it isn't built in, but since it's built into dash, bash and zsh, that's not much of a concern in the context of this script).
I find echo clearer than printf since you don't have to worry about escape sequences. They each have their downside: echo only works portably on strings that don't contain backslashes and don't start with a dash, but it's straightforward for those strings. printf is well-defined but you need to remember about its escapes (and it's slower in some shells where it isn't built in, but since it's built into dash, bash and zsh, that's not much of a concern in the context of this script).
| printf "script should also be adjusted.\n" | ||
| printf "\n" | ||
|
|
||
| cat <<EOF |
ronald-cron-arm
Sep 24, 2020
Contributor
Is that a well-known shell construct? Why not just some echo's there?
Is that a well-known shell construct? Why not just some echo's there?
gilles-peskine-arm
Sep 24, 2020
Author
Contributor
This is the idiomatic shell construct to print multiple lines. https://stackoverflow.com/questions/2953081/how-can-i-write-a-heredoc-to-a-file-in-bash-script
This is the idiomatic shell construct to print multiple lines. https://stackoverflow.com/questions/2953081/how-can-i-write-a-heredoc-to-a-file-in-bash-script
|
Thanks for the clarifications, LGTM. |
|
The CI failure in head job is a CI infrastructure glitch and all merge job tests passed thus good to go. |
ronald-cron-arm
merged commit efcf52d
into
ARMmbed:development
Fix a minor bug in
ssl-opt.shthat caused spurious messages on standard error and missing information in log files, but no test failure and no missed detection of a test failure. The bug was triggered by GnuTLS ciphersuite priority specifications containing%and manifested in errors likeThe bug was calling
printf "$foo"instead ofprintf %s "$foo". Fix other instances of this mistake elsewhere, even if it had no practical consequence with current data.The commit 880f7f2 “ssl-opt.sh --help: don't show regexps for -f and -e” is shared with #3615 to avoid a conflict between the two PR.
Backports: #3828, #3829