pkgsrc-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[pkgsrc/trunk]: pkgsrc mk/tools: fix quoting when logging tool invocations
details: https://anonhg.NetBSD.org/pkgsrc/rev/d70e64d65a28
branches: trunk
changeset: 331721:d70e64d65a28
user: rillig <rillig%pkgsrc.org@localhost>
date: Sun Mar 24 08:40:07 2019 +0000
description:
mk/tools: fix quoting when logging tool invocations
When a package or the infrastructure defined a tool with custom
TOOLS_ARGS or TOOLS_SCRIPT containing special characters, these could
lead to unintuitive interactions at the time when that tool invocation
was logged in the tool wrapper log. Some of the logging output ended up
on stdout, while some of the normal output ended up in the log, and parts
of the quoted arguments were even evaluated as shell commands.
The logging of the wrapped tool commands is not perfect yet, but at least
it's much more predictable now.
diffstat:
mk/tools/create.mk | 20 ++++++++++-----
regress/tools/Makefile | 14 ++++++++++-
regress/tools/files/logging-test.sh | 47 ++++++++++++++++++++++++++++--------
3 files changed, 62 insertions(+), 19 deletions(-)
diffs (184 lines):
diff -r 9af0557f5771 -r d70e64d65a28 mk/tools/create.mk
--- a/mk/tools/create.mk Sun Mar 24 07:44:55 2019 +0000
+++ b/mk/tools/create.mk Sun Mar 24 08:40:07 2019 +0000
@@ -1,4 +1,4 @@
-# $NetBSD: create.mk,v 1.9 2019/03/22 22:13:21 rillig Exp $
+# $NetBSD: create.mk,v 1.10 2019/03/24 08:40:07 rillig Exp $
#
# Copyright (c) 2005, 2006 The NetBSD Foundation, Inc.
# All rights reserved.
@@ -137,8 +137,6 @@
TOOLS_PATH.${_t_}?= ${FALSE}
TOOLS_SCRIPT_DFLT.${_t_}= \
${TOOLS_PATH.${_t_}} ${TOOLS_ARGS.${_t_}} "$$@"
-_TOOLS_LOGSCRIPT_DFLT.${_t_}= \
- ${TOOLS_PATH.${_t_}} ${TOOLS_ARGS.${_t_}} $$*
override-tools: ${TOOLS_CMD.${_t_}}
@@ -151,18 +149,24 @@
if ${TEST} -n ${TOOLS_SCRIPT.${_t_}:Q}""; then \
create=wrapper; \
script=${TOOLS_SCRIPT.${_t_}:Q}; \
- logscript="$$script"; \
+ logprefix='"set args "$$@"; shift; "'; \
+ logmain=${TOOLS_SCRIPT.${_t_}:Q:Q}; \
+ logsuffix=''; \
elif ${TEST} -n ${TOOLS_PATH.${_t_}:Q}""; then \
if ${TEST} -n ${TOOLS_ARGS.${_t_}:Q}""; then \
create=wrapper; \
script=${TOOLS_SCRIPT_DFLT.${_t_}:Q}; \
- logscript=${_TOOLS_LOGSCRIPT_DFLT.${_t_}:Q}; \
+ logprefix=''; \
+ logmain=${TOOLS_PATH.${_t_}:Q:Q}\"\ \"${TOOLS_ARGS.${_t_}:Q:Q}; \
+ logsuffix=' "$$*"'; \
else \
case ${TOOLS_PATH.${_t_}:Q}"" in \
/*) create=symlink ;; \
*) create=wrapper; \
script=${TOOLS_SCRIPT_DFLT.${_t_}:Q}; \
- logscript=${_TOOLS_LOGSCRIPT_DFLT.${_t_}:Q}; \
+ logprefix=''; \
+ logmain=${TOOLS_PATH.${_t_}:Q:Q}\"\ \"; \
+ logsuffix=' "$$*"'; \
esac; \
fi; \
else \
@@ -173,7 +177,9 @@
{ ${ECHO} '#!'${TOOLS_SHELL:Q}; \
${ECHO} 'wrapperlog="$${TOOLS_WRAPPER_LOG-'${_TOOLS_WRAP_LOG:Q}'}"'; \
${ECHO} '${ECHO} "[*] "'${.TARGET:Q}'" $$*" >> $$wrapperlog'; \
- ${ECHO} "${ECHO} \"<.> $$logscript\" >> \$$wrapperlog"; \
+ ${ECHO} 'logprefix='$$logprefix; \
+ ${ECHO} 'logmain='$$logmain; \
+ ${ECHO} "${ECHO} '<.>' \"\$$logprefix\$$logmain\"$$logsuffix >> \$$wrapperlog"; \
${ECHO} "$$script"; \
} > ${.TARGET:Q}; \
${CHMOD} +x ${.TARGET:Q}; \
diff -r 9af0557f5771 -r d70e64d65a28 regress/tools/Makefile
--- a/regress/tools/Makefile Sun Mar 24 07:44:55 2019 +0000
+++ b/regress/tools/Makefile Sun Mar 24 08:40:07 2019 +0000
@@ -1,4 +1,4 @@
-# $NetBSD: Makefile,v 1.13 2019/03/23 22:59:11 rillig Exp $
+# $NetBSD: Makefile,v 1.14 2019/03/24 08:40:07 rillig Exp $
#
DISTNAME= # not applicable
@@ -43,6 +43,18 @@
done; \
printf '\n'
+# Demonstrates that double quotes in both the TOOLS_ARGS and the actual
+# arguments are properly logged.
+TOOLS_CREATE+= path-args-dquot
+TOOLS_PATH.path-args-dquot= echo
+TOOLS_ARGS.path-args-dquot= \" "\"" '"'
+
+# Demonstrates that both the TOOLS_ARGS and the actual arguments are
+# properly logged.
+TOOLS_CREATE+= path-args
+TOOLS_PATH.path-args= echo
+TOOLS_ARGS.path-args= " \"'\\$$" "*"
+
do-build:
.for t in ${REGRESS_TESTS}
${RUN} cd ${WRKSRC}; \
diff -r 9af0557f5771 -r d70e64d65a28 regress/tools/files/logging-test.sh
--- a/regress/tools/files/logging-test.sh Sun Mar 24 07:44:55 2019 +0000
+++ b/regress/tools/files/logging-test.sh Sun Mar 24 08:40:07 2019 +0000
@@ -1,5 +1,5 @@
#! /bin/sh
-# $NetBSD: logging-test.sh,v 1.4 2019/03/23 22:59:11 rillig Exp $
+# $NetBSD: logging-test.sh,v 1.5 2019/03/24 08:40:08 rillig Exp $
# Up to March 2019, the command logging for the wrapped tools didn't properly
# quote the command line arguments. This meant the logging did not reflect
@@ -64,6 +64,8 @@
# argument. This is because the echo command doesn't get any
# additional arguments by the tool wrapper (TOOLS_ARGS.echo).
+ # TODO: To avoid unintended file expansion when replaying the
+ # commands, all arguments must be shquoted.
assert_log <<'EOF'
[*] WRKDIR/.tools/bin/echo begin * * * end
<.> echo begin * * * end
@@ -92,20 +94,35 @@
EOF
}
+test_case "TOOLS_PATH with TOOLS_ARGS containing double quotes"
+{
+ run_tool path-args-dquot "and" \" "\"" '"'
+
+ assert_log <<'EOF'
+[*] WRKDIR/.tools/bin/path-args-dquot and " " "
+<.> echo \" "\"" '"' and " " "
+EOF
+}
+
+test_case "TOOLS_PATH with TOOLS_ARGS containing special characters"
+{
+ run_tool path-args "and" " \"'\\$" "*"
+
+ assert_log <<'EOF'
+[*] WRKDIR/.tools/bin/path-args and "'\$ *
+<.> echo " \"'\\$" "*" and "'\$ *
+EOF
+}
+
test_case "TOOLS_SCRIPT with dquot"
{
run_tool script-dquot
# The following log output contains a trailing whitespace. This
# is because the tool didn't get any actual arguments.
- #
- # FIXME: the "echo oops" occurs because the script is not
- # properly quoted during logging.
assert_log <<'EOF'
[*] WRKDIR/.tools/bin/script-dquot
-[*] WRKDIR/.tools/bin/world
-<.> echo oops
-oops
+<.> set args ; shift; echo "hello; world"
EOF
}
@@ -117,7 +134,7 @@
# is because the tool didn't get any actual arguments.
assert_log <<'EOF'
[*] WRKDIR/.tools/bin/script-backslash
-<.> echo hello\;\ world
+<.> set args ; shift; echo hello\;\ world
EOF
}
@@ -125,10 +142,18 @@
{
run_tool for-loop "one" "two" "three"
- # TODO: Add proper quoting for the printf argument inside the loop.
+ # The actual command is written to the log in a form as close as
+ # possible to replay it. Since the command may do anything with
+ # its arguments, it's the safest way to set them first and then
+ # just log the command verbatim.
+ #
+ # In this example, the $0 becomes unrealistic when the command
+ # is replayed. In practice $0 is seldom used.
+ #
+ # TODO: In the "set arg", the arguments must be shquoted.
assert_log <<'EOF'
[*] WRKDIR/.tools/bin/for-loop one two three
-<.> printf '%s' WRKDIR/.tools/bin/for-loop; for arg in one two three; do printf ' <%s>' ; done; printf '\n'
+<.> set args one two three; shift; printf '%s' "$0"; for arg in "$@"; do printf ' <%s>' "$arg"; done; printf '\n'
EOF
}
@@ -143,6 +168,6 @@
# TODO: Add proper quoting for the arguments.
assert_log <<'EOF'
[*] WRKDIR/.tools/bin/for-loop -DSD="a b" -DSS='a b' -DDD="a b" -DB="a b"
-<.> printf '%s' WRKDIR/.tools/bin/for-loop; for arg in -DSD="a b" -DSS='a b' -DDD="a b" -DB="a b"; do printf ' <%s>' ; done; printf '\n'
+<.> set args -DSD="a b" -DSS='a b' -DDD="a b" -DB="a b"; shift; printf '%s' "$0"; for arg in "$@"; do printf ' <%s>' "$arg"; done; printf '\n'
EOF
}
Home |
Main Index |
Thread Index |
Old Index