On Thu, Apr 7, 2016 at 12:27 PM, Dan Nicholson <nichol...@endlessm.com> wrote:
> On Thu, Apr 7, 2016 at 11:56 AM, Felipe Sateler <fsate...@debian.org> wrote:
>>
>> AFAICT, the only difference between quotewords and split is that
>> quotewords takes into account quotes to allow "multiple words" to be
>> parsed as a single token. I don't think we have use for such a feature
>> (there are no such files in the archive).
>>
>> So, unless I'm missing something, this could be further simplified to
>> just use split.
>
> Yeah, that does seem possible. The only fields parsed are WantedBy,
> RequiredBy, Also and Alias. Since the values here are unit names,
> there shouldn't be any instances where there's valid whitespace in the
> words. So, I think you could do split followed by quote stripping and
> everything would be fine. I'll try that out.
>
> Of course, if init-system-helpers ever does parse any fields might
> contain contain multiple quoted words, then you'd need to go back to
> using quotewords or something like that.
The attached patch seems to work in my testing.
--
Dan
From 504671d973c2779e968bb975e844d0d04d6e67c9 Mon Sep 17 00:00:00 2001
From: Dan Nicholson <nichol...@endlessm.com>
Date: Thu, 7 Apr 2016 11:08:23 -0700
Subject: [PATCH] script: Handle \ escapes in unit names properly
Unit names can contain valid \ escapes in systemd. See systemd-escape(1)
for examples. Text::ParseWords::shellwords breaks that since it strips \
in addition to quotes. Instead, just use split as the only fields parsed
will contain unit names, which can't have whitespace. Any
leading/trailing quotes are then stripped separately.
Closes: #820359
---
debian/changelog | 7 +++++++
script/deb-systemd-helper | 14 ++++++++++----
script/dh_systemd_enable | 1 -
script/dh_systemd_start | 11 +++++++++--
4 files changed, 26 insertions(+), 7 deletions(-)
diff --git a/debian/changelog b/debian/changelog
index e5e0ce5..01c1433 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -11,6 +11,13 @@ init-system-helpers (1.30) UNRELEASED; urgency=medium
* dh_systemd_*: Add DH promise to avoid being called for no reason.
* Update Vcs-* fields to use https.
+ [ Dan Nicholson ]
+ * deb-systemd-helper, dh_systemd_start: Use split rather than
+ Text::ParseWords::shellwords since the latter will strip valid \
+ escapes from unit names. The values then need to have leading and
+ trailing quotes stripped. (Closes: #820359)
+ * dh_systemd_enable: Drop unused Text::ParseWords use.
+
-- Felipe Sateler <fsate...@debian.org> Fri, 11 Mar 2016 18:48:39 -0300
init-system-helpers (1.29) unstable; urgency=medium
diff --git a/script/deb-systemd-helper b/script/deb-systemd-helper
index 2a87cb4..7241555 100755
--- a/script/deb-systemd-helper
+++ b/script/deb-systemd-helper
@@ -85,7 +85,6 @@ use warnings;
use File::Path qw(make_path); # in core since Perl 5.001
use File::Basename; # in core since Perl 5
use File::Temp qw(tempfile); # in core since Perl 5.6.1
-use Text::ParseWords qw(shellwords); # in core since Perl 5
use Getopt::Long; # in core since Perl 5
# Make Data::Dumper::Dumper available if present (not present on systems that
# only have perl-base, not perl).
@@ -183,13 +182,18 @@ sub get_link_closure {
my $unit_name = basename($service_path);
+ # The keys parsed from the unit file below can only have unit names
+ # as values. Since unit names can't have whitespace in systemd,
+ # simply use split and strip any leading/trailing quotes. See
+ # systemd-escape(1) for examples of valid unit names.
open my $fh, '<', $service_path or error("unable to read $service_path");
while (my $line = <$fh>) {
chomp($line);
my $service_link;
if ($line =~ /^\s*(WantedBy|RequiredBy)=(.+)$/i) {
- for my $value (shellwords($2)) {
+ for my $value (split(/\s+/, $2)) {
+ $value =~ s/^(["'])(.*)\g1$/$2/;
my $wants_dir = "/etc/systemd/system/$value";
$wants_dir .= '.wants' if $1 eq 'WantedBy';
$wants_dir .= '.requires' if $1 eq 'RequiredBy';
@@ -198,7 +202,8 @@ sub get_link_closure {
}
if ($line =~ /^\s*Also=(.+)$/i) {
- for my $value (shellwords($1)) {
+ for my $value (split(/\s+/, $1)) {
+ $value =~ s/^(["'])(.*)\g1$/$2/;
if ($value ne $unit_name) {
push @links, get_link_closure($value, find_unit($value));
}
@@ -206,7 +211,8 @@ sub get_link_closure {
}
if ($line =~ /^\s*Alias=(.+)$/i) {
- for my $value (shellwords($1)) {
+ for my $value (split(/\s+/, $1)) {
+ $value =~ s/^(["'])(.*)\g1$/$2/;
if ($value ne $unit_name) {
push @links, { dest => $service_path, src => "/etc/systemd/system/$1" };
}
diff --git a/script/dh_systemd_enable b/script/dh_systemd_enable
index 39955c4..a45615b 100755
--- a/script/dh_systemd_enable
+++ b/script/dh_systemd_enable
@@ -9,7 +9,6 @@ dh_systemd_enable - enable/disable systemd unit files
use strict;
use Debian::Debhelper::Dh_Lib;
use File::Find;
-use Text::ParseWords qw(shellwords); # in core since Perl 5
=head1 SYNOPSIS
diff --git a/script/dh_systemd_start b/script/dh_systemd_start
index 4e22d59..5d996ca 100755
--- a/script/dh_systemd_start
+++ b/script/dh_systemd_start
@@ -9,7 +9,6 @@ dh_systemd_start - start/stop/restart systemd unit files
use strict;
use Debian::Debhelper::Dh_Lib;
use File::Find;
-use Text::ParseWords qw(shellwords); # in core since Perl 5
use Cwd qw(getcwd abs_path);
=head1 SYNOPSIS
@@ -107,8 +106,16 @@ sub extract_key {
while (my $line = <$fh>) {
chomp($line);
+ # The keys parsed from the unit file below can only have
+ # unit names as values. Since unit names can't have
+ # whitespace in systemd, simply use split and strip any
+ # leading/trailing quotes. See systemd-escape(1) for
+ # examples of valid unit names.
if ($line =~ /^\s*$key=(.+)$/i) {
- @values = (@values, shellwords($1));
+ for my $value (split(/\s+/, $1)) {
+ $value =~ s/^(["'])(.*)\g1$/$2/;
+ push @values, $value;
+ }
}
}
close($fh);
--
2.1.4
