Bug#946055: /etc/etckeeper/commit.d/50vcs-commit[22]: ${@#-m}: bad substitution
On 2019-12-18 22:59:54, Thorsten Glaser wrote: >> I encourage you to engage with joeyh anyways, if you can. ;) > > We engaged in real life during our trip to DebConf in Bosnia, > which was enjoyable (if a lengthy road trip), I can relate ;) > I just had trouble with the website (it’s not my medium anyway). They might welcome patches by email... -- Hard times are coming when we will be wanting the voices of writers who can see alternatives to how we live now and can see through our fear-stricken society and its obsessive technologies to other ways of being, and even imagine some real grounds for hope. We will need writers who can remember freedom. Poets, visionaries—the realists of a larger reality. - Ursula Le Guin
Bug#946055: /etc/etckeeper/commit.d/50vcs-commit[22]: ${@#-m}: bad substitution
On Wed, 18 Dec 2019, Antoine Beaupré wrote: > It's not a static page: there's a field at the top of the page where you > can input a page name and then you get served a login form and so on. Ah, okay. Perhaps it didn’t show up in my browser or so. > But nevermind that, I filed it: > > https://etckeeper.branchable.com/todo/vcs-commit_hook_is_not_POSIX-friendly/ Thanks! > I encourage you to engage with joeyh anyways, if you can. ;) We engaged in real life during our trip to DebConf in Bosnia, which was enjoyable (if a lengthy road trip), I can relate ;) I just had trouble with the website (it’s not my medium anyway). bye, //mirabilos -- tarent solutions GmbH Rochusstraße 2-4, D-53123 Bonn • http://www.tarent.de/ Tel: +49 228 54881-393 • Fax: +49 228 54881-235 HRB 5168 (AG Bonn) • USt-ID (VAT): DE122264941 Geschäftsführer: Dr. Stefan Barth, Kai Ebenrett, Boris Esser, Alexander Steeg ** Mit der tarent Academy bieten wir auch Trainings und Schulungen in den Bereichen Softwareentwicklung, Agiles Arbeiten und Zukunftstechnologien an. Besuchen Sie uns auf www.tarent.de/academy. Wir freuen uns auf Ihren Kontakt. **
Bug#946055: /etc/etckeeper/commit.d/50vcs-commit[22]: ${@#-m}: bad substitution
On 2019-12-18 20:42:11, Thorsten Glaser wrote: >> https://etckeeper.branchable.com/todo/ > > This seems to be a static page? I don’t find out how to do that. It's not a static page: there's a field at the top of the page where you can input a page name and then you get served a login form and so on. But nevermind that, I filed it: https://etckeeper.branchable.com/todo/vcs-commit_hook_is_not_POSIX-friendly/ I encourage you to engage with joeyh anyways, if you can. ;) a. -- Dr. King’s major assumption was that if you are nonviolent, if you suffer, your opponent will see your suffering and will be moved to change his heart. He only made one fallacious assumption: In order for nonviolence to work, your opponent must have a conscience. The United States has none.- Stokely Carmichael
Bug#946055: /etc/etckeeper/commit.d/50vcs-commit[22]: ${@#-m}: bad substitution
Control: forward -1 https://etckeeper.branchable.com/todo/vcs-commit_hook_is_not_POSIX-friendly/ On 2019-12-18 20:32:41, Thorsten Glaser wrote: > On Wed, 18 Dec 2019, Thorsten Glaser wrote: > >> The patch fixes the issue by reimplementing the -mmessage feature >> introduced in 1.18.11 in portable shell *and* fixing issues with >> messages starting with -n or -c or containing backslashes as well. > > I can confirm that a package built with this patch fixes the issue. > > The debugging sponsored by ⮡ tarent, see below. > > I’ve not been able to figure out how to submit *either* patches > *or* bugreports upstream. Please do so (DevRef §5.8.3(6)). Ack. Forwarded. a. -- The value of a college education is not the learning of many facts but the training of the mind to think. - Albert Einstein
Bug#946055: /etc/etckeeper/commit.d/50vcs-commit[22]: ${@#-m}: bad substitution
Hi Antoine,
> This package, like all of mine, is LowNMU so upload whenever you feel
> ready.
OK, thanks!
> > ++ sed '1s/^-m \{0,1\}//' >"$logfile" <<-EOF
> > ++ $*
> > ++ EOF
>
> ... isn't there a simpler way than piping this through sed?
I’ve wondered about this. There are several ways, for example
involving printf(1) and manipulating $1 in shell, but they all
amount to more code and not better either.
> does that regex really do the same as ${@#-m}? it seems there's some
> whitespace and anchoring stuff going on here that would subtly change
> behavior...
${@#-m} is not specified what it does. The intent of the patch is
to remove either '-m' as first parameter or the leading '-m' from
the first parameter, then concatenate all parameters and write
them into the logfile.
What my code does is: first concatenate. This uses the first byte
in $IFS, which is a space, so we have '$*' expand to '$1 $2 $3 …'
(of course no spaces if the next parameters are not set). Then it
strips a leading either '-m' (second case above) or '-m ' (first
case above: -m as a parameter of its own) from *ONLY* the first
line.
Consider this:
etckeeper commit -m 'foo bar
-mbaz'
The user clearly expects the second line to start with “-m”.
Use of a here document instead of echo also fixes the following:
etckeeper commit -m -n foo
etckeeper commit -m -c foo
etckeeper commit -m 'foo\tbar'
(The behaviour of echo(1) is also not specified, or rather,
implementation-defined, if the arguments start with a hyphen-minus
or contain a backslash.)
> https://etckeeper.branchable.com/todo/
This seems to be a static page? I don’t find out how to do that.
> Let me know if you want me to carry that for you.
That would be great, thanks!
bye,
//mirabilos
--
tarent solutions GmbH
Rochusstraße 2-4, D-53123 Bonn • http://www.tarent.de/
Tel: +49 228 54881-393 • Fax: +49 228 54881-235
HRB 5168 (AG Bonn) • USt-ID (VAT): DE122264941
Geschäftsführer: Dr. Stefan Barth, Kai Ebenrett, Boris Esser, Alexander Steeg
**
Mit der tarent Academy bieten wir auch Trainings und Schulungen in den
Bereichen Softwareentwicklung, Agiles Arbeiten und Zukunftstechnologien an.
Besuchen Sie uns auf www.tarent.de/academy. Wir freuen uns auf Ihren Kontakt.
**
Bug#946055: /etc/etckeeper/commit.d/50vcs-commit[22]: ${@#-m}: bad substitution
On Wed, 18 Dec 2019, Thorsten Glaser wrote: > The patch fixes the issue by reimplementing the -mmessage feature > introduced in 1.18.11 in portable shell *and* fixing issues with > messages starting with -n or -c or containing backslashes as well. I can confirm that a package built with this patch fixes the issue. The debugging sponsored by ⮡ tarent, see below. I’ve not been able to figure out how to submit *either* patches *or* bugreports upstream. Please do so (DevRef §5.8.3(6)). bye, //mirabilos -- tarent solutions GmbH Rochusstraße 2-4, D-53123 Bonn • http://www.tarent.de/ Tel: +49 228 54881-393 • Fax: +49 228 54881-235 HRB 5168 (AG Bonn) • USt-ID (VAT): DE122264941 Geschäftsführer: Dr. Stefan Barth, Kai Ebenrett, Boris Esser, Alexander Steeg
Bug#946055: /etc/etckeeper/commit.d/50vcs-commit[22]: ${@#-m}: bad substitution
On 2019-12-18 20:16:50, Thorsten Glaser wrote:
> Package: etckeeper
> Version: 1.18.12-1
> Followup-For: Bug #946055
>
> Find a patch attached. I intend to NMU if this isn’t fixed RSN.
This package, like all of mine, is LowNMU so upload whenever you feel
ready.
> The patch fixes the issue by reimplementing the -mmessage feature
> introduced in 1.18.11 in portable shell *and* fixing issues with
> messages starting with -n or -c or containing backslashes as well.
Thanks for the patch! I do have a comment though...
> diff -Nru etckeeper-1.18.12/debian/patches/fix-sh-syntax.diff
> etckeeper-1.18.12/debian/patches/fix-sh-syntax.diff
> --- etckeeper-1.18.12/debian/patches/fix-sh-syntax.diff 1970-01-01
> 01:00:00.0 +0100
> +++ etckeeper-1.18.12/debian/patches/fix-sh-syntax.diff 2019-12-18
> 20:11:17.0 +0100
> @@ -0,0 +1,19 @@
> +# DP: fix #946055 by reimplementing in portable shell
> +# DP: also fixes issues with echo -n, -c, and backslashes
> +
> +--- a/commit.d/50vcs-commit
> b/commit.d/50vcs-commit
> +@@ -12,10 +12,9 @@ if [ -n "$1" ]; then
> + if [ "x$1" = "x--stdin" ]; then
> + cat > "$logfile"
> + else
> +-if [ "x$1" = "x-m" ]; then
> +-shift 1
> +-fi
> +-echo "${@#-m}" > "$logfile"
> ++sed '1s/^-m \{0,1\}//' >"$logfile" <<-EOF
> ++$*
> ++EOF
... isn't there a simpler way than piping this through sed?
does that regex really do the same as ${@#-m}? it seems there's some
whitespace and anchoring stuff going on here that would subtly change
behavior...
I also encourage you to submit your patch upstream in:
https://etckeeper.branchable.com/
specifically here:
https://etckeeper.branchable.com/todo/
Let me know if you want me to carry that for you.
Thanks again!
a.
--
If quantum mechanics hasn't profoundly shocked you, you haven't
understood it yet.
- Niels Bohr
Bug#946055: /etc/etckeeper/commit.d/50vcs-commit[22]: ${@#-m}: bad substitution
Package: etckeeper
Version: 1.18.12-1
Followup-For: Bug #946055
Find a patch attached. I intend to NMU if this isn’t fixed RSN.
The patch fixes the issue by reimplementing the -mmessage feature
introduced in 1.18.11 in portable shell *and* fixing issues with
messages starting with -n or -c or containing backslashes as well.
-- System Information:
Debian Release: bullseye/sid
APT prefers buildd-unstable
APT policy: (500, 'buildd-unstable'), (500, 'unstable'), (1, 'experimental')
Architecture: amd64 (x86_64)
Foreign Architectures: i386
Kernel: Linux 5.3.0-3-amd64 (SMP w/2 CPU cores)
Kernel taint flags: TAINT_WARN
Locale: LANG=C, LC_CTYPE=C.UTF-8 (charmap=UTF-8), LANGUAGE=C (charmap=UTF-8)
Shell: /bin/sh linked to /bin/lksh
Init: sysvinit (via /sbin/init)
Versions of packages etckeeper depends on:
ii debconf [debconf-2.0] 1.5.73
ii git1:2.24.1-1
Versions of packages etckeeper recommends:
ii cron [cron-daemon] 3.0pl1-135
Versions of packages etckeeper suggests:
ii sudo 1.8.29-1
-- debconf information:
etckeeper/purge: true
diff -Nru etckeeper-1.18.12/debian/changelog etckeeper-1.18.12/debian/changelog
--- etckeeper-1.18.12/debian/changelog 2019-12-01 21:08:30.0 +0100
+++ etckeeper-1.18.12/debian/changelog 2019-12-18 20:11:25.0 +0100
@@ -1,3 +1,13 @@
+etckeeper (1.18.12-1.1) unstable; urgency=medium
+
+ * Non-maintainer upload.
+ * debian/patches/fix-sh-syntax.diff: new (Closes: #946055)
+- reimplements -mmessage from 1.18.11 in portable shell
+- fixes potential bugs with messages starting with hyphen-minus
+ (in particular -n, -c) and containing backslashes
+
+ -- Thorsten Glaser Wed, 18 Dec 2019 20:11:25 +0100
+
etckeeper (1.18.12-1) unstable; urgency=medium
[ Antoine Beaupré ]
diff -Nru etckeeper-1.18.12/debian/patches/fix-sh-syntax.diff
etckeeper-1.18.12/debian/patches/fix-sh-syntax.diff
--- etckeeper-1.18.12/debian/patches/fix-sh-syntax.diff 1970-01-01
01:00:00.0 +0100
+++ etckeeper-1.18.12/debian/patches/fix-sh-syntax.diff 2019-12-18
20:11:17.0 +0100
@@ -0,0 +1,19 @@
+# DP: fix #946055 by reimplementing in portable shell
+# DP: also fixes issues with echo -n, -c, and backslashes
+
+--- a/commit.d/50vcs-commit
b/commit.d/50vcs-commit
+@@ -12,10 +12,9 @@ if [ -n "$1" ]; then
+ if [ "x$1" = "x--stdin" ]; then
+ cat > "$logfile"
+ else
+- if [ "x$1" = "x-m" ]; then
+- shift 1
+- fi
+- echo "${@#-m}" > "$logfile"
++ sed '1s/^-m \{0,1\}//' >"$logfile" <<-EOF
++ $*
++ EOF
+ fi
+ else
+ logfile=""
diff -Nru etckeeper-1.18.12/debian/patches/series
etckeeper-1.18.12/debian/patches/series
--- etckeeper-1.18.12/debian/patches/series 1970-01-01 01:00:00.0
+0100
+++ etckeeper-1.18.12/debian/patches/series 2019-12-18 20:02:00.0
+0100
@@ -0,0 +1 @@
+fix-sh-syntax.diff
Bug#946055: /etc/etckeeper/commit.d/50vcs-commit[22]: ${@#-m}: bad substitution
Package: etckeeper
Version: 1.18.12-1
Severity: serious
Justification: Policy 10.4
/etc/etckeeper/commit.d/50vcs-commit[22]: ${@#-m}: bad substitution
This is because doing trim operations on an array are not implemented
in mksh, and unspecified in POSIX:
[…] If parameter is '#', '*', or '@', the result of the expansion is
unspecified. […]
cf.
https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_06_02
-- System Information:
Debian Release: bullseye/sid
APT prefers unstable-debug
APT policy: (500, 'unstable-debug'), (500, 'buildd-unstable'), (500,
'unstable'), (1, 'experimental')
Architecture: amd64 (x86_64)
Kernel: Linux 5.2.0-3-amd64 (SMP w/4 CPU cores)
Kernel taint flags: TAINT_FIRMWARE_WORKAROUND
Locale: LANG=C, LC_CTYPE=en_GB.UTF-8 (charmap=UTF-8), LANGUAGE=C (charmap=UTF-8)
Shell: /bin/sh linked to /bin/lksh
Init: sysvinit (via /sbin/init)
Versions of packages etckeeper depends on:
ii debconf [debconf-2.0] 1.5.73
ii git1:2.24.0-1
ii mercurial 5.2-1
Versions of packages etckeeper recommends:
pn cron-daemon
Versions of packages etckeeper suggests:
ii sudo 1.8.29-1
-- debconf information:
etckeeper/purge: true
