[pacman-dev] [PATCH] makepkg: Don't interpret format specifiers in msg
Message string containing for example windows paths would have unexpected behaviour. For example the message "Check C:\foo\bar" would be printed as "Check C:<newline> oar". Signed-off-by: Niklas Holm <jadedcyborg@gmail.com> --- scripts/libmakepkg/util/message.sh.in | 15 +++++---------- scripts/library/output_format.sh | 18 ++++++------------ 2 files changed, 11 insertions(+), 22 deletions(-) diff --git a/scripts/libmakepkg/util/message.sh.in b/scripts/libmakepkg/util/message.sh.in index 33808de7..90799640 100644 --- a/scripts/libmakepkg/util/message.sh.in +++ b/scripts/libmakepkg/util/message.sh.in @@ -44,26 +44,21 @@ colorize() { } plain() { - local mesg=$1; shift - printf "${BOLD} ${mesg}${ALL_OFF}\n" "$@" >&2 + printf "${BOLD} %s${ALL_OFF}\n" "$1" >&2 } msg() { - local mesg=$1; shift - printf "${GREEN}==>${ALL_OFF}${BOLD} ${mesg}${ALL_OFF}\n" "$@" >&2 + printf "${GREEN}==>${ALL_OFF}${BOLD} %s${ALL_OFF}\n" "$1" >&2 } msg2() { - local mesg=$1; shift - printf "${BLUE} ->${ALL_OFF}${BOLD} ${mesg}${ALL_OFF}\n" "$@" >&2 + printf "${BLUE} ->${ALL_OFF}${BOLD} %s${ALL_OFF}\n" "$1" >&2 } warning() { - local mesg=$1; shift - printf "${YELLOW}==> $(gettext "WARNING:")${ALL_OFF}${BOLD} ${mesg}${ALL_OFF}\n" "$@" >&2 + printf "${YELLOW}==> $(gettext "WARNING:")${ALL_OFF}${BOLD} %s${ALL_OFF}\n" "$1" >&2 } error() { - local mesg=$1; shift - printf "${RED}==> $(gettext "ERROR:")${ALL_OFF}${BOLD} ${mesg}${ALL_OFF}\n" "$@" >&2 + printf "${RED}==> $(gettext "ERROR:")${ALL_OFF}${BOLD} %s${ALL_OFF}\n" "$1" >&2 } diff --git a/scripts/library/output_format.sh b/scripts/library/output_format.sh index 9f02c00b..2049aab3 100644 --- a/scripts/library/output_format.sh +++ b/scripts/library/output_format.sh @@ -1,32 +1,26 @@ plain() { (( QUIET )) && return - local mesg=$1; shift - printf "${BOLD} ${mesg}${ALL_OFF}\n" "$@" >&1 + printf "${BOLD} %s${ALL_OFF}\n" "$1" >&1 } msg() { (( QUIET )) && return - local mesg=$1; shift - printf "${GREEN}==>${ALL_OFF}${BOLD} ${mesg}${ALL_OFF}\n" "$@" >&1 + printf "${GREEN}==>${ALL_OFF}${BOLD} %s${ALL_OFF}\n" "$1" >&1 } msg2() { (( QUIET )) && return - local mesg=$1; shift - printf "${BLUE} ->${ALL_OFF}${BOLD} ${mesg}${ALL_OFF}\n" "$@" >&1 + printf "${BLUE} ->${ALL_OFF}${BOLD} %s${ALL_OFF}\n" "$1" >&1 } ask() { - local mesg=$1; shift - printf "${BLUE}::${ALL_OFF}${BOLD} ${mesg}${ALL_OFF}" "$@" >&1 + printf "${BLUE}::${ALL_OFF}${BOLD} %s${ALL_OFF}" "$1" >&1 } warning() { - local mesg=$1; shift - printf "${YELLOW}==> $(gettext "WARNING:")${ALL_OFF}${BOLD} ${mesg}${ALL_OFF}\n" "$@" >&2 + printf "${YELLOW}==> $(gettext "WARNING:")${ALL_OFF}${BOLD} %s${ALL_OFF}\n" "$1" >&2 } error() { - local mesg=$1; shift - printf "${RED}==> $(gettext "ERROR:")${ALL_OFF}${BOLD} ${mesg}${ALL_OFF}\n" "$@" >&2 + printf "${RED}==> $(gettext "ERROR:")${ALL_OFF}${BOLD} %s${ALL_OFF}\n" "$1" >&2 } -- 2.16.2
On 23/02/18 20:42, Niklas Holm wrote:
Message string containing for example windows paths would have unexpected behaviour. For example the message "Check C:\foo\bar" would be printed as "Check C:<newline> oar".
Signed-off-by: Niklas Holm <jadedcyborg@gmail.com>
Did you test this patch?
On 23/02/18 21:18, Allan McRae wrote:
On 23/02/18 20:42, Niklas Holm wrote:
Message string containing for example windows paths would have unexpected behaviour. For example the message "Check C:\foo\bar" would be printed as "Check C:<newline> oar".
Signed-off-by: Niklas Holm <jadedcyborg@gmail.com>
Did you test this patch? .
Try something like: msg() { local mesg=$1; shift printf -v mesg2 "${mesg}" "$@" printf "${GREEN}==>${ALL_OFF}${BOLD} %s${ALL_OFF}\n" "${mesg2}">&2 }
On 02/23/2018 05:42 AM, Niklas Holm wrote:
Message string containing for example windows paths would have unexpected behaviour. For example the message "Check C:\foo\bar" would be printed as "Check C:<newline> oar".
I think you're missing the purpose of these functions. They are wrappers for printf, so that you can use them the same way you would use printf. That means that Windows paths and other things containing escape codes are meant to be interjected into msg the same way you would interject them into printf... msg "Check %s" "C:\foo\bar" This is working *exactly* as intended. You patch breaks that, because now we cannot provide gettext'ized $mesg arguments that contain %s format strings. -- Eli Schwartz Bug Wrangler and Trusted User
On 23/02/18 21:55, Eli Schwartz wrote:
On 02/23/2018 05:42 AM, Niklas Holm wrote:
Message string containing for example windows paths would have unexpected behaviour. For example the message "Check C:\foo\bar" would be printed as "Check C:<newline> oar".
I think you're missing the purpose of these functions. They are wrappers for printf, so that you can use them the same way you would use printf.
That means that Windows paths and other things containing escape codes are meant to be interjected into msg the same way you would interject them into printf...
msg "Check %s" "C:\foo\bar"
This is working *exactly* as intended. You patch breaks that, because now we cannot provide gettext'ized $mesg arguments that contain %s format strings.
My question is, is the example purely hypothetical, or do we pass a path not using a %s variable somewhere in the code base. A
participants (3)
-
Allan McRae
-
Eli Schwartz
-
Niklas Holm