Merge lp:~cjwatson/launchpad-buildd/run-non-ascii-arguments into lp:launchpad-buildd

Proposed by Colin Watson on 2019-06-09
Status: Merged
Merged at revision: 384
Proposed branch: lp:~cjwatson/launchpad-buildd/run-non-ascii-arguments
Merge into: lp:launchpad-buildd
Diff against target: 127 lines (+45/-0)
5 files modified
debian/changelog (+2/-0)
lpbuildd/target/chroot.py (+6/-0)
lpbuildd/target/lxd.py (+5/-0)
lpbuildd/target/tests/test_chroot.py (+17/-0)
lpbuildd/target/tests/test_lxd.py (+15/-0)
To merge this branch: bzr merge lp:~cjwatson/launchpad-buildd/run-non-ascii-arguments
Reviewer Review Type Date Requested Status
Adam Collard (community) Approve on 2019-06-10
Launchpad code reviewers 2019-06-09 Pending
Review via email: mp+368588@code.launchpad.net

Commit message

Encode non-bytes subprocess arguments on Python 2.

Description of the change

This avoids crashing on non-ASCII file names under LC_CTYPE=C.

To post a comment you must log in.
Adam Collard (adam-collard) wrote :

+1

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'debian/changelog'
2--- debian/changelog 2019-06-05 14:10:20 +0000
3+++ debian/changelog 2019-06-09 07:50:37 +0000
4@@ -2,6 +2,8 @@
5
6 * Allow configuring APT or snap store proxies via a new [proxy]
7 configuration file section.
8+ * Encode non-bytes subprocess arguments on Python 2 to avoid crashing on
9+ non-ASCII file names under LC_CTYPE=C (LP: #1832072).
10
11 -- Colin Watson <cjwatson@ubuntu.com> Wed, 05 Jun 2019 15:06:54 +0100
12
13
14=== modified file 'lpbuildd/target/chroot.py'
15--- lpbuildd/target/chroot.py 2019-01-10 18:14:55 +0000
16+++ lpbuildd/target/chroot.py 2019-06-09 07:50:37 +0000
17@@ -11,6 +11,8 @@
18 import subprocess
19 import time
20
21+import six
22+
23 from lpbuildd.target.backend import (
24 Backend,
25 BackendException,
26@@ -77,6 +79,10 @@
27 if echo:
28 print("Running in chroot: %s" % ' '.join(
29 shell_escape(arg) for arg in args))
30+ if six.PY2:
31+ # The behaviour of non-bytes subprocess arguments in Python 2
32+ # depends on the interpreter's startup locale.
33+ args = [arg.encode("UTF-8") for arg in args]
34 cmd = ["sudo", "/usr/sbin/chroot", self.chroot_path] + args
35 if input_text is None and not get_output:
36 subprocess.check_call(cmd, **kwargs)
37
38=== modified file 'lpbuildd/target/lxd.py'
39--- lpbuildd/target/lxd.py 2019-01-10 18:14:55 +0000
40+++ lpbuildd/target/lxd.py 2019-06-09 07:50:37 +0000
41@@ -20,6 +20,7 @@
42 import netaddr
43 import pylxd
44 from pylxd.exceptions import LXDAPIException
45+import six
46
47 from lpbuildd.target.backend import (
48 Backend,
49@@ -490,6 +491,10 @@
50 if echo:
51 print("Running in container: %s" % ' '.join(
52 shell_escape(arg) for arg in args))
53+ if six.PY2:
54+ # The behaviour of non-bytes subprocess arguments in Python 2
55+ # depends on the interpreter's startup locale.
56+ args = [arg.encode("UTF-8") for arg in args]
57 # pylxd's Container.execute doesn't support sending stdin, and it's
58 # tedious to implement ourselves.
59 cmd = ["lxc", "exec", self.name] + env_params + ["--"] + args
60
61=== modified file 'lpbuildd/target/tests/test_chroot.py'
62--- lpbuildd/target/tests/test_chroot.py 2019-01-10 18:14:55 +0000
63+++ lpbuildd/target/tests/test_chroot.py 2019-06-09 07:50:37 +0000
64@@ -13,6 +13,7 @@
65 EnvironmentVariable,
66 TempDir,
67 )
68+import six
69 from systemfixtures import (
70 FakeFilesystem,
71 FakeProcesses,
72@@ -117,6 +118,22 @@
73 expected_args,
74 [proc._args["args"] for proc in processes_fixture.procs])
75
76+ def test_run_non_ascii_arguments(self):
77+ self.useFixture(EnvironmentVariable("HOME", "/expected/home"))
78+ processes_fixture = self.useFixture(FakeProcesses())
79+ processes_fixture.add(lambda _: {}, name="sudo")
80+ arg = u"\N{SNOWMAN}"
81+ Chroot("1", "xenial", "amd64").run(["echo", arg])
82+
83+ expected_args = [
84+ ["sudo", "/usr/sbin/chroot",
85+ "/expected/home/build-1/chroot-autobuild",
86+ "linux64", "echo", arg.encode("UTF-8") if six.PY2 else arg],
87+ ]
88+ self.assertEqual(
89+ expected_args,
90+ [proc._args["args"] for proc in processes_fixture.procs])
91+
92 def test_copy_in(self):
93 self.useFixture(EnvironmentVariable("HOME", "/expected/home"))
94 source_dir = self.useFixture(TempDir()).path
95
96=== modified file 'lpbuildd/target/tests/test_lxd.py'
97--- lpbuildd/target/tests/test_lxd.py 2019-01-10 18:14:55 +0000
98+++ lpbuildd/target/tests/test_lxd.py 2019-06-09 07:50:37 +0000
99@@ -25,6 +25,7 @@
100 )
101 import pylxd
102 from pylxd.exceptions import LXDAPIException
103+import six
104 from systemfixtures import (
105 FakeFilesystem,
106 FakeProcesses,
107@@ -530,6 +531,20 @@
108 expected_args,
109 [proc._args["args"] for proc in processes_fixture.procs])
110
111+ def test_run_non_ascii_arguments(self):
112+ processes_fixture = self.useFixture(FakeProcesses())
113+ processes_fixture.add(lambda _: {}, name="lxc")
114+ arg = u"\N{SNOWMAN}"
115+ LXD("1", "xenial", "amd64").run(["echo", arg])
116+
117+ expected_args = [
118+ ["lxc", "exec", "lp-xenial-amd64", "--",
119+ "linux64", "echo", arg.encode("UTF-8") if six.PY2 else arg],
120+ ]
121+ self.assertEqual(
122+ expected_args,
123+ [proc._args["args"] for proc in processes_fixture.procs])
124+
125 def test_copy_in(self):
126 source_dir = self.useFixture(TempDir()).path
127 self.useFixture(MockPatch("pylxd.Client"))

Subscribers

People subscribed via source and target branches

to all changes: