Merge lp:~jamesodhunt/upstart/bugs-1235649+1203595 into lp:upstart

Proposed by James Hunt on 2013-10-18
Status: Rejected
Rejected by: Steve Langasek on 2013-10-23
Proposed branch: lp:~jamesodhunt/upstart/bugs-1235649+1203595
Merge into: lp:upstart
Diff against target: 1044 lines (+730/-34)
13 files modified
ChangeLog (+48/-1)
init/control.c (+45/-12)
init/control.h (+5/-1)
init/environ.c (+4/-0)
init/job_class.c (+56/-6)
init/job_process.c (+1/-1)
init/main.c (+12/-5)
init/tests/test_control.c (+2/-2)
init/tests/test_environ.c (+457/-0)
test/test_util_common.c (+11/-1)
util/initctl.c (+2/-3)
util/man/initctl.8 (+2/-2)
util/tests/test_initctl.c (+85/-0)
To merge this branch: bzr merge lp:~jamesodhunt/upstart/bugs-1235649+1203595
Reviewer Review Type Date Requested Status
Steve Langasek 2013-10-18 Disapprove on 2013-10-23
Review via email: mp+191852@code.launchpad.net

Description of the change

Make a Session Init connect to the D-Bus session bus as well as the private socket.

To post a comment you must log in.
James Hunt (jamesodhunt) wrote :

This branch makes the Session Init attempt to connect to the D-Bus session bus on startup and retry when sent SIGUSR1. This behaviour is consistent with Upstart running as PID 1 and connecting to the D-Bus system bus.

The branch achieves the required behaviour by setting all global job environment variables in the Session Inits own environment too. This is specifically so that the Session Init will have knowledge of DBUS_SESSION_BUS_ADDRESS but avoids hard-coding a whitelist of variables to set. Note that since the Session Inits environment will change with each 'initctl (un)set-env -g ...', a subsequent call to 'initctl reset-env -g' will set the environment back to contain:

1) The default variables (PATH, TERM).
2) The Session Inits initial environment.
3) Any variables set via 'set-env -g'.

This is a subtle behavioural change which although not ideal can be rectified by manually running 'unsetenv $var' for all job global variables that should be removed from job environments.

Note that Upstart running as PID 1 will not have its environment modified.

Tested to ensure that bug 1235649 is resolved when a client connects to Upstart via the D-Bus session bus without a main loop and the spam.sh (attached to the bug) is run.

James Hunt (jamesodhunt) wrote :

Discussion with slangasek has resulted in a better plan to solve this issue.

Steve Langasek (vorlon) wrote :

I was assuming you would replace this with a fresh branch, based on our discussion, since it will have almost nothing in common with the current code. :) So rejecting this mp.

As we discussed, a better approach is to make this explicit rather than implicit by having a new initctl interface that takes the dbus address as an argument. When a session init receives this call, it should connect to the dbus session bus at the specified address if it isn't already connected to the session bus. (Requests to connect to a session bus if there's already a session bus should be ignored.) This way, we don't have to fiddle with the environment at all.

review: Disapprove

Unmerged revisions

1545. By James Hunt on 2013-10-22

* init/control.c: control_bus_open(): Don't call nih_dbus_bus() if
  DBUS_SESSION_BUS_ADDRESS is not set to avoid D-bus auto-launching a
  dbus-daemon.
* init/environ.c: Comments.
* init/job_class.c:
  - job_class_environment_init(): Superior check on whether job_environ
    is not empty.
  - job_class_environment_reset(): Only reset job_environ if not NULL
    already.
  - job_class_environment_set(): Set variable in Upstarts environment
    too (required to allow Upstart to be aware of the D-Bus session bus
    address when the dbus-daemon is available).
  - job_class_environment_unset(): Unset variable from Upstarts
    environment, but only if it is not a default variable.
* init/job_process.c: Formatting.
* init/test_control.c: Updated strings used by tests which check error
  messages to include 'D-Bus'.
* init/test_environ.c:
  - test_add(): New test:
    - "using bare word with no corresponding variable set in environment"
  - test_remove(): New function ("the missing test") containing 8 new tests:
    - "remove name=value pair with empty table"
    - "remove bare name with empty table"
    - "remove name=value from table of size 1"
    - "remove bare name from table of size 1"
    - "remove first name=value entry from table of size 2"
    - "remove first bare name entry from table of size 2"
    - "remove last name=value entry from table of size 2"
    - "remove last bare name entry from table of size 2"
* test/test_util_common.c:
  - Formatting.
  - get_initctl(): Added environment checks.
* util/initctl.c:
  - Formatting.
  - Removed testing comment from option text for '--session'.
* util/man/initctl.8: Removed testing comment for '--session'.
* util/tests/test_initctl.c:
  - test_session_init(): New test that checks the Session Init now
    connects to the D-Bus session bus.

1544. By James Hunt on 2013-10-18

* Connect to the D-Bus session bus as well as using a private socket
  when running as a Session Init (LP: #1203595, #1235649).

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'ChangeLog'
2--- ChangeLog 2013-10-04 21:34:25 +0000
3+++ ChangeLog 2013-10-22 10:15:27 +0000
4@@ -1,4 +1,51 @@
5-2013-01-04 Steve Langasek <steve.langasek@ubuntu.com
6+2013-10-22 James Hunt <james.hunt@ubuntu.com>
7+
8+ * init/control.c: control_bus_open(): Don't call nih_dbus_bus() if
9+ DBUS_SESSION_BUS_ADDRESS is not set to avoid D-bus auto-launching a
10+ dbus-daemon.
11+ * init/environ.c: Comments.
12+ * init/job_class.c:
13+ - job_class_environment_init(): Superior check on whether job_environ
14+ is not empty.
15+ - job_class_environment_reset(): Only reset job_environ if not NULL
16+ already.
17+ - job_class_environment_set(): Set variable in Upstarts environment
18+ too (required to allow Upstart to be aware of the D-Bus session bus
19+ address when the dbus-daemon is available).
20+ - job_class_environment_unset(): Unset variable from Upstarts
21+ environment, but only if it is not a default variable.
22+ * init/job_process.c: Formatting.
23+ * init/test_control.c: Updated strings used by tests which check error
24+ messages to include 'D-Bus'.
25+ * init/test_environ.c:
26+ - test_add(): New test:
27+ - "using bare word with no corresponding variable set in environment"
28+ - test_remove(): New function ("the missing test") containing 8 new tests:
29+ - "remove name=value pair with empty table"
30+ - "remove bare name with empty table"
31+ - "remove name=value from table of size 1"
32+ - "remove bare name from table of size 1"
33+ - "remove first name=value entry from table of size 2"
34+ - "remove first bare name entry from table of size 2"
35+ - "remove last name=value entry from table of size 2"
36+ - "remove last bare name entry from table of size 2"
37+ * test/test_util_common.c:
38+ - Formatting.
39+ - get_initctl(): Added environment checks.
40+ * util/initctl.c:
41+ - Formatting.
42+ - Removed testing comment from option text for '--session'.
43+ * util/man/initctl.8: Removed testing comment for '--session'.
44+ * util/tests/test_initctl.c:
45+ - test_session_init(): New test that checks the Session Init now
46+ connects to the D-Bus session bus.
47+
48+2013-10-18 James Hunt <james.hunt@ubuntu.com>
49+
50+ * Connect to the D-Bus session bus as well as using a private socket
51+ when running as a Session Init (LP: #1203595, #1235649).
52+
53+2013-10-04 Steve Langasek <steve.langasek@ubuntu.com
54
55 * extra/upstart-local-bridge.c: use SOCKET_PATH in our event
56 environment, instead of clobbering PATH. (LP: #1235480)
57
58=== modified file 'init/control.c'
59--- init/control.c 2013-04-22 10:30:09 +0000
60+++ init/control.c 2013-10-22 10:15:27 +0000
61@@ -81,11 +81,19 @@
62 *
63 * If TRUE, connect to the D-Bus session bus rather than the system bus.
64 *
65- * Used for testing.
66+ * Used for testing to simulate (as far as possible) a system-like init
67+ * when running as a non-priv user.
68 **/
69 int use_session_bus = FALSE;
70
71 /**
72+ * dbus_bus_type:
73+ *
74+ * Type of D-Bus bus to connect to.
75+ **/
76+DBusBusType dbus_bus_type;
77+
78+/**
79 * control_server_address:
80 *
81 * Address on which the control server may be reached.
82@@ -258,16 +266,35 @@
83
84 control_init ();
85
86- control_handle_bus_type ();
87-
88- /* Connect to the D-Bus System Bus and hook everything up into
89+ dbus_bus_type = control_get_bus_type ();
90+
91+ /* Avoid D-Bus attempting to autolaunch a dbus-daemon since:
92+ *
93+ * - the behaviour is undesirable: launching a dbus-daemon
94+ * should be handled by a job.
95+ *
96+ * - if autolaunch fails (for example if DISPLAY is not set, or
97+ * dbus-uuidgen has not been run (or the generated file is
98+ * unreadable)), libdbus may call abort(), depending on how
99+ * libdbus has been built.
100+ */
101+ if (dbus_bus_type == DBUS_BUS_SESSION && ! getenv ("DBUS_SESSION_BUS_ADDRESS")) {
102+ nih_dbus_error_raise ("dbus session bus error",
103+ "DBUS_SESSION_BUS_ADDRESS not set");
104+ return -1;
105+ }
106+
107+ /* Connect to the appropriate D-Bus bus and hook everything up into
108 * our own main loop automatically.
109 */
110- conn = nih_dbus_bus (use_session_bus ? DBUS_BUS_SESSION : DBUS_BUS_SYSTEM,
111- control_disconnected);
112+ conn = nih_dbus_bus (dbus_bus_type, control_disconnected);
113 if (! conn)
114 return -1;
115
116+ nih_debug ("Connected to D-Bus %s bus",
117+ dbus_bus_type == DBUS_BUS_SESSION
118+ ? "session" : "system");
119+
120 /* Register objects on the bus. */
121 control_register_all (conn);
122
123@@ -345,7 +372,9 @@
124
125 dbus_error_init (&error);
126
127- nih_warn (_("Disconnected from system bus"));
128+ nih_warn (_("Disconnected from D-Bus %s bus"),
129+ dbus_bus_type == DBUS_BUS_SESSION
130+ ? "session" : "system");
131
132 control_bus = NULL;
133 }
134@@ -817,19 +846,23 @@
135 }
136
137 /**
138- * control_handle_bus_type:
139+ * control_get_bus_type:
140 *
141 * Determine D-Bus bus type to connect to.
142+ *
143+ * Returns: Type of D-Bus bus to connect to.
144 **/
145-void
146-control_handle_bus_type (void)
147+DBusBusType
148+control_get_bus_type (void)
149 {
150 if (getenv (USE_SESSION_BUS_ENV))
151 use_session_bus = TRUE;
152
153- if (use_session_bus)
154- nih_debug ("Using session bus");
155+ return (use_session_bus || user_mode)
156+ ? DBUS_BUS_SESSION
157+ : DBUS_BUS_SYSTEM;
158 }
159+
160 /**
161 * control_notify_disk_writeable:
162 * @data: not used,
163
164=== modified file 'init/control.h'
165--- init/control.h 2013-05-08 16:21:08 +0000
166+++ init/control.h 2013-10-22 10:15:27 +0000
167@@ -134,7 +134,11 @@
168 const char *log_priority)
169 __attribute__ ((warn_unused_result));
170
171-void control_handle_bus_type (void);
172+DBusBusType control_get_bus_type (void)
173+ __attribute__ ((warn_unused_result));
174+
175+const char *control_get_bus_name (void)
176+ __attribute__ ((warn_unused_result));
177
178 void control_prepare_reexec (void);
179
180
181=== modified file 'init/environ.c'
182--- init/environ.c 2013-01-08 15:57:31 +0000
183+++ init/environ.c 2013-10-22 10:15:27 +0000
184@@ -202,6 +202,10 @@
185 for (e = *env; e && *e; e++) {
186 keylen = strcspn (*e, "=");
187
188+ /* Found @str in the existing environment (either as a
189+ * name=value pair, or a bare name), so don't copy it to
190+ * the new environment.
191+ */
192 if (! strncmp (str, *e, keylen))
193 continue;
194
195
196=== modified file 'init/job_class.c'
197--- init/job_class.c 2013-07-22 00:15:43 +0000
198+++ init/job_class.c 2013-10-22 10:15:27 +0000
199@@ -89,6 +89,14 @@
200 NihHash *job_classes = NULL;
201
202 /**
203+ * default_environ:
204+ *
205+ * Minimal set of environment variables all jobs are provided with by
206+ * default.
207+ */
208+static char * const default_environ[] = { JOB_DEFAULT_ENVIRONMENT, NULL };
209+
210+/**
211 * job_environ:
212 *
213 * Array of environment variables that will be set in the jobs
214@@ -116,8 +124,6 @@
215 void
216 job_class_environment_init (void)
217 {
218- char * const default_environ[] = { JOB_DEFAULT_ENVIRONMENT, NULL };
219-
220 if (job_environ)
221 return;
222
223@@ -125,7 +131,7 @@
224 NIH_MUST (environ_append (&job_environ, NULL, 0, TRUE, default_environ));
225
226 if (user_mode && ! no_inherit_env)
227- NIH_MUST(environ_append (&job_environ, NULL, 0, TRUE, environ));
228+ NIH_MUST (environ_append (&job_environ, NULL, 0, TRUE, environ));
229 }
230
231 /**
232@@ -138,10 +144,10 @@
233 void
234 job_class_environment_reset (void)
235 {
236- if (job_environ)
237+ if (job_environ) {
238 nih_free (job_environ);
239-
240- job_environ = NULL;
241+ job_environ = NULL;
242+ }
243
244 job_class_environment_init ();
245 }
246@@ -160,9 +166,31 @@
247 int
248 job_class_environment_set (const char *var, int replace)
249 {
250+ nih_local char *name = NULL;
251+ char *value;
252+
253 nih_assert (var);
254 nih_assert (job_environ);
255
256+ name = nih_strdup (NULL, var);
257+ if (! name)
258+ return -1;
259+
260+ value = strchr (name, '=');
261+ nih_assert (value);
262+ *value = '\0';
263+
264+ /* Jump over delimiter */
265+ value++;
266+
267+ /* Set variable in Upstarts environment.
268+ *
269+ * This isn't necessary for jobs (due to job_environ), but is
270+ * required to allow the Session Init to connect to the D-Bus
271+ * session bus.
272+ */
273+ setenv (name, value, replace);
274+
275 if (! environ_add (&job_environ, NULL, NULL, replace, var))
276 return -1;
277
278@@ -193,9 +221,20 @@
279 int
280 job_class_environment_unset (const char *name)
281 {
282+ char * const *e;
283+ int keep = FALSE;
284+
285 nih_assert (name);
286 nih_assert (job_environ);
287
288+ /* Determine if @name is a default variable */
289+ for (e = default_environ; e && *e; e++) {
290+ if (! strcmp (*e, name)) {
291+ keep = TRUE;
292+ break;
293+ }
294+ }
295+
296 if (! environ_remove (&job_environ, NULL, NULL, name))
297 return -1;
298
299@@ -211,6 +250,17 @@
300 }
301 }
302
303+ if (! keep) {
304+ /* Remove variable from Upstarts environment, but only if it is
305+ * not part of the default environment; if we did this,
306+ * it would not be possible to reset the job environment
307+ * since adding a variable by name would *not* be added
308+ * to the job environment table since the environment
309+ * lookup would fail.
310+ */
311+ unsetenv (name);
312+ }
313+
314 return 0;
315 }
316
317
318=== modified file 'init/job_process.c'
319--- init/job_process.c 2013-10-03 14:43:24 +0000
320+++ init/job_process.c 2013-10-22 10:15:27 +0000
321@@ -278,7 +278,7 @@
322 env = NIH_MUST (nih_str_array_new (NULL));
323
324 if (job->env)
325- NIH_MUST(environ_append (&env, NULL, &envc, TRUE, job->env));
326+ NIH_MUST (environ_append (&env, NULL, &envc, TRUE, job->env));
327
328 if (job->stop_env
329 && ((process == PROCESS_PRE_STOP)
330
331=== modified file 'init/main.c'
332--- init/main.c 2013-07-31 09:28:48 +0000
333+++ init/main.c 2013-10-22 10:15:27 +0000
334@@ -128,7 +128,7 @@
335 extern int default_console;
336 extern int write_state_file;
337 extern char *log_dir;
338-
339+extern DBusBusType dbus_bus_type;
340
341 /**
342 * options:
343@@ -213,7 +213,8 @@
344 if (disable_job_logging)
345 nih_debug ("Job logging disabled");
346
347- control_handle_bus_type ();
348+ if (getenv (USE_SESSION_BUS_ENV))
349+ use_session_bus = TRUE;
350
351 if (! user_mode)
352 no_inherit_env = TRUE;
353@@ -933,14 +934,20 @@
354 NihSignal *signal)
355 {
356 if (! control_bus) {
357- nih_info (_("Reconnecting to system bus"));
358+ char *dbus_bus_name;
359+
360+ dbus_bus_name = dbus_bus_type == DBUS_BUS_SESSION
361+ ? "session" : "system";
362+
363+ nih_info (_("Reconnecting to D-Bus %s bus"),
364+ dbus_bus_name);
365
366 if (control_bus_open () < 0) {
367 NihError *err;
368
369 err = nih_error_get ();
370- nih_warn ("%s: %s", _("Unable to connect to the system bus"),
371- err->message);
372+ nih_warn (_("Unable to connect to the D-Bus %s bus: %s"),
373+ dbus_bus_name, err->message);
374 nih_free (err);
375 }
376 }
377
378=== modified file 'init/tests/test_control.c'
379--- init/tests/test_control.c 2012-09-09 21:27:24 +0000
380+++ init/tests/test_control.c 2013-10-22 10:15:27 +0000
381@@ -821,7 +821,7 @@
382
383 TEST_LIST_EMPTY (control_conns);
384
385- TEST_FILE_EQ (output, "test: Disconnected from system bus\n");
386+ TEST_FILE_EQ (output, "test: Disconnected from D-Bus system bus\n");
387 TEST_FILE_END (output);
388 TEST_FILE_RESET (output);
389
390@@ -879,7 +879,7 @@
391
392 TEST_LIST_EMPTY (control_conns);
393
394- TEST_FILE_EQ (output, "test: Disconnected from system bus\n");
395+ TEST_FILE_EQ (output, "test: Disconnected from D-Bus system bus\n");
396 TEST_FILE_END (output);
397 TEST_FILE_RESET (output);
398
399
400=== modified file 'init/tests/test_environ.c'
401--- init/tests/test_environ.c 2011-06-06 17:05:11 +0000
402+++ init/tests/test_environ.c 2013-10-22 10:15:27 +0000
403@@ -572,6 +572,462 @@
404 }
405
406 unsetenv ("BAR");
407+
408+ /* Check that attempting to add a variable by name fails if
409+ * there is no corresponding environment variable set.
410+ */
411+ TEST_FEATURE ("using bare word with no corresponding variable set in environment");
412+
413+ /* Ensure variable not set initially */
414+ TEST_EQ_P (getenv ("UPSTART_TEST_VARIABLE"), NULL);
415+
416+ TEST_ALLOC_FAIL {
417+ TEST_ALLOC_SAFE {
418+ len = 0;
419+ env = nih_str_array_new (NULL);
420+ assert (nih_str_array_add (&env, NULL, &len,
421+ "FOO=BAR"));
422+ }
423+
424+ ret = environ_add (&env, NULL, &len, FALSE, "UPSTART_TEST_VARIABLE");
425+
426+ if (test_alloc_failed) {
427+ TEST_EQ_P (ret, NULL);
428+
429+ TEST_EQ (len, 1);
430+ TEST_EQ_STR (env[0], "FOO=BAR");
431+ TEST_EQ_P (env[1], NULL);
432+
433+ nih_free (env);
434+ continue;
435+ }
436+
437+ /* XXX: Attempting to add an unset variable results in
438+ * no change to the table (it is not an error!)
439+ */
440+ TEST_EQ_P (ret, env);
441+
442+ TEST_EQ (len, 1);
443+ TEST_EQ_STR (env[0], "FOO=BAR");
444+ TEST_EQ_P (env[1], NULL);
445+
446+ nih_free (env);
447+ }
448+}
449+
450+void
451+test_remove (void)
452+{
453+ char **env = NULL, **ret;
454+ size_t len = 0;
455+
456+ TEST_FUNCTION ("environ_remove");
457+
458+ TEST_FEATURE ("remove name=value pair with empty table");
459+ TEST_ALLOC_FAIL {
460+ TEST_ALLOC_SAFE {
461+ len = 0;
462+ env = nih_str_array_new (NULL);
463+ }
464+
465+ ret = environ_remove (&env, NULL, &len, "FOO=BAR");
466+
467+ if (test_alloc_failed) {
468+ TEST_EQ_P (ret, NULL);
469+
470+ TEST_EQ (len, 0);
471+ TEST_EQ_P (env[0], NULL);
472+
473+ nih_free (env);
474+ continue;
475+ }
476+
477+ TEST_EQ_P (ret, NULL);
478+ TEST_EQ (len, 0);
479+ TEST_EQ_P (env[0], NULL);
480+
481+ nih_free (env);
482+ }
483+
484+ TEST_FEATURE ("remove bare name with empty table");
485+ TEST_ALLOC_FAIL {
486+ TEST_ALLOC_SAFE {
487+ len = 0;
488+ env = nih_str_array_new (NULL);
489+ }
490+
491+ ret = environ_remove (&env, NULL, &len, "FOO");
492+
493+ if (test_alloc_failed) {
494+ TEST_EQ_P (ret, NULL);
495+
496+ TEST_EQ (len, 0);
497+ TEST_EQ_P (env[0], NULL);
498+
499+ nih_free (env);
500+ continue;
501+ }
502+
503+ TEST_EQ_P (ret, NULL);
504+ TEST_EQ (len, 0);
505+ TEST_EQ_P (env[0], NULL);
506+
507+ nih_free (env);
508+ }
509+
510+ TEST_FEATURE ("remove name=value from table of size 1");
511+ TEST_ALLOC_FAIL {
512+ TEST_ALLOC_SAFE {
513+ len = 0;
514+ env = nih_str_array_new (NULL);
515+
516+ ret = environ_add (&env, NULL, &len, TRUE, "FOO=BAR");
517+ TEST_NE_P (ret, NULL);
518+
519+ TEST_EQ (len, 1);
520+ TEST_ALLOC_PARENT (env[0], env);
521+ TEST_ALLOC_SIZE (env[0], 8);
522+ TEST_EQ_STR (env[0], "FOO=BAR");
523+ TEST_EQ_P (env[1], NULL);
524+ }
525+
526+ ret = environ_remove (&env, NULL, &len, "FOO=BAR");
527+
528+ if (test_alloc_failed) {
529+ TEST_EQ_P (ret, NULL);
530+
531+ TEST_EQ (len, 1);
532+
533+ TEST_ALLOC_PARENT (env[0], env);
534+ TEST_ALLOC_SIZE (env[0], 8);
535+
536+ TEST_NE_P (env[0], NULL);
537+ TEST_EQ_STR (env[0], "FOO=BAR");
538+
539+ TEST_EQ_P (env[1], NULL);
540+
541+ nih_free (env);
542+ continue;
543+ }
544+
545+ TEST_NE_P (ret, NULL);
546+ TEST_EQ (len, 0);
547+ TEST_EQ_P (env[0], NULL);
548+
549+ nih_free (env);
550+ }
551+
552+ TEST_FEATURE ("remove bare name from table of size 1");
553+ TEST_ALLOC_FAIL {
554+ TEST_ALLOC_SAFE {
555+ len = 0;
556+ env = nih_str_array_new (NULL);
557+
558+ ret = environ_add (&env, NULL, &len, TRUE, "FOO=BAR");
559+ TEST_NE_P (ret, NULL);
560+
561+ TEST_EQ (len, 1);
562+ TEST_ALLOC_PARENT (env[0], env);
563+ TEST_ALLOC_SIZE (env[0], 8);
564+ TEST_EQ_STR (env[0], "FOO=BAR");
565+ TEST_EQ_P (env[1], NULL);
566+ }
567+
568+ ret = environ_remove (&env, NULL, &len, "FOO");
569+
570+ if (test_alloc_failed) {
571+ TEST_EQ_P (ret, NULL);
572+
573+ TEST_EQ (len, 1);
574+
575+ TEST_ALLOC_PARENT (env[0], env);
576+ TEST_ALLOC_SIZE (env[0], 8);
577+
578+ TEST_NE_P (env[0], NULL);
579+ TEST_EQ_STR (env[0], "FOO=BAR");
580+
581+ TEST_EQ_P (env[1], NULL);
582+
583+ nih_free (env);
584+ continue;
585+ }
586+
587+ TEST_NE_P (ret, NULL);
588+ TEST_EQ (len, 0);
589+ TEST_EQ_P (env[0], NULL);
590+
591+ nih_free (env);
592+ }
593+
594+ TEST_FEATURE ("remove first name=value entry from table of size 2");
595+ TEST_ALLOC_FAIL {
596+ TEST_ALLOC_SAFE {
597+ len = 0;
598+ env = nih_str_array_new (NULL);
599+
600+ ret = environ_add (&env, NULL, &len, TRUE, "FOO=BAR");
601+ TEST_NE_P (ret, NULL);
602+
603+ TEST_EQ (len, 1);
604+ TEST_ALLOC_PARENT (env[0], env);
605+ TEST_ALLOC_SIZE (env[0], 8);
606+ TEST_EQ_STR (env[0], "FOO=BAR");
607+ TEST_EQ_P (env[1], NULL);
608+
609+ ret = environ_add (&env, NULL, &len, TRUE, "BAZ=QUX");
610+ TEST_NE_P (ret, NULL);
611+
612+ TEST_EQ (len, 2);
613+ TEST_ALLOC_PARENT (env[0], env);
614+ TEST_ALLOC_SIZE (env[0], 8);
615+ TEST_EQ_STR (env[0], "FOO=BAR");
616+
617+ TEST_ALLOC_PARENT (env[1], env);
618+ TEST_ALLOC_SIZE (env[1], 8);
619+ TEST_EQ_STR (env[1], "BAZ=QUX");
620+
621+ TEST_EQ_P (env[2], NULL);
622+ }
623+
624+ /* Remove first entry added */
625+ ret = environ_remove (&env, NULL, &len, "FOO=BAR");
626+
627+ if (test_alloc_failed) {
628+ TEST_EQ_P (ret, NULL);
629+
630+ TEST_EQ (len, 2);
631+ TEST_ALLOC_PARENT (env[0], env);
632+ TEST_ALLOC_SIZE (env[0], 8);
633+ TEST_EQ_STR (env[0], "FOO=BAR");
634+
635+ TEST_ALLOC_PARENT (env[1], env);
636+ TEST_ALLOC_SIZE (env[1], 8);
637+ TEST_EQ_STR (env[1], "BAZ=QUX");
638+
639+ TEST_EQ_P (env[2], NULL);
640+
641+ nih_free (env);
642+ continue;
643+ }
644+
645+ TEST_NE_P (ret, NULL);
646+ TEST_EQ (len, 1);
647+
648+ TEST_ALLOC_PARENT (env[0], env);
649+ TEST_ALLOC_SIZE (env[0], 8);
650+ TEST_EQ_STR (env[0], "BAZ=QUX");
651+
652+ TEST_EQ_P (env[1], NULL);
653+
654+ nih_free (env);
655+ }
656+
657+ TEST_FEATURE ("remove first bare name entry from table of size 2");
658+
659+ /* Set a variable to allow the bare name to be expanded */
660+ assert0 (setenv ("UPSTART_TEST_VARIABLE", "foo", 1));
661+
662+ TEST_ALLOC_FAIL {
663+ TEST_ALLOC_SAFE {
664+ len = 0;
665+ env = nih_str_array_new (NULL);
666+
667+ ret = environ_add (&env, NULL, &len, TRUE, "UPSTART_TEST_VARIABLE");
668+ TEST_NE_P (ret, NULL);
669+
670+ TEST_EQ (len, 1);
671+ TEST_ALLOC_PARENT (env[0], env);
672+ TEST_ALLOC_SIZE (env[0], 8);
673+
674+ /* Should have been expanded */
675+ TEST_EQ_STR (env[0], "UPSTART_TEST_VARIABLE=foo");
676+
677+ TEST_EQ_P (env[1], NULL);
678+
679+ ret = environ_add (&env, NULL, &len, TRUE, "BAZ=QUX");
680+ TEST_NE_P (ret, NULL);
681+
682+ TEST_EQ (len, 2);
683+
684+ TEST_ALLOC_PARENT (env[0], env);
685+ TEST_ALLOC_SIZE (env[0], 8);
686+ TEST_EQ_STR (env[0], "UPSTART_TEST_VARIABLE=foo");
687+
688+ TEST_ALLOC_PARENT (env[1], env);
689+ TEST_ALLOC_SIZE (env[1], 8);
690+ TEST_EQ_STR (env[1], "BAZ=QUX");
691+
692+ TEST_EQ_P (env[2], NULL);
693+ }
694+
695+ /* Remove first entry added */
696+ ret = environ_remove (&env, NULL, &len, "UPSTART_TEST_VARIABLE");
697+
698+ if (test_alloc_failed) {
699+ TEST_EQ_P (ret, NULL);
700+
701+ TEST_EQ (len, 2);
702+ TEST_ALLOC_PARENT (env[0], env);
703+ TEST_ALLOC_SIZE (env[0], 8);
704+ TEST_EQ_STR (env[0], "UPSTART_TEST_VARIABLE=foo");
705+
706+ TEST_ALLOC_PARENT (env[1], env);
707+ TEST_ALLOC_SIZE (env[1], 8);
708+ TEST_EQ_STR (env[1], "BAZ=QUX");
709+
710+ TEST_EQ_P (env[2], NULL);
711+
712+ nih_free (env);
713+ continue;
714+ }
715+
716+ TEST_NE_P (ret, NULL);
717+ TEST_EQ (len, 1);
718+
719+ TEST_ALLOC_PARENT (env[0], env);
720+ TEST_ALLOC_SIZE (env[0], 8);
721+ TEST_EQ_STR (env[0], "BAZ=QUX");
722+
723+ TEST_EQ_P (env[1], NULL);
724+
725+ nih_free (env);
726+ }
727+
728+ assert0 (unsetenv ("UPSTART_TEST_VARIABLE"));
729+
730+ TEST_FEATURE ("remove last name=value entry from table of size 2");
731+ TEST_ALLOC_FAIL {
732+ TEST_ALLOC_SAFE {
733+ len = 0;
734+ env = nih_str_array_new (NULL);
735+
736+ ret = environ_add (&env, NULL, &len, TRUE, "FOO=BAR");
737+ TEST_NE_P (ret, NULL);
738+
739+ TEST_EQ (len, 1);
740+ TEST_ALLOC_PARENT (env[0], env);
741+ TEST_ALLOC_SIZE (env[0], 8);
742+ TEST_EQ_STR (env[0], "FOO=BAR");
743+ TEST_EQ_P (env[1], NULL);
744+
745+ ret = environ_add (&env, NULL, &len, TRUE, "BAZ=QUX");
746+ TEST_NE_P (ret, NULL);
747+
748+ TEST_EQ (len, 2);
749+ TEST_ALLOC_PARENT (env[0], env);
750+ TEST_ALLOC_SIZE (env[0], 8);
751+ TEST_EQ_STR (env[0], "FOO=BAR");
752+
753+ TEST_ALLOC_PARENT (env[1], env);
754+ TEST_ALLOC_SIZE (env[1], 8);
755+ TEST_EQ_STR (env[1], "BAZ=QUX");
756+
757+ TEST_EQ_P (env[2], NULL);
758+ }
759+
760+ /* Remove last entry added */
761+ ret = environ_remove (&env, NULL, &len, "BAZ=QUX");
762+
763+ if (test_alloc_failed) {
764+ TEST_EQ_P (ret, NULL);
765+
766+ TEST_EQ (len, 2);
767+ TEST_ALLOC_PARENT (env[0], env);
768+ TEST_ALLOC_SIZE (env[0], 8);
769+ TEST_EQ_STR (env[0], "FOO=BAR");
770+
771+ TEST_ALLOC_PARENT (env[1], env);
772+ TEST_ALLOC_SIZE (env[1], 8);
773+ TEST_EQ_STR (env[1], "BAZ=QUX");
774+
775+ TEST_EQ_P (env[2], NULL);
776+
777+ nih_free (env);
778+ continue;
779+ }
780+
781+ TEST_NE_P (ret, NULL);
782+ TEST_EQ (len, 1);
783+
784+ TEST_ALLOC_PARENT (env[0], env);
785+ TEST_ALLOC_SIZE (env[0], 8);
786+ TEST_EQ_STR (env[0], "FOO=BAR");
787+
788+ TEST_EQ_P (env[1], NULL);
789+
790+ nih_free (env);
791+ }
792+
793+ TEST_FEATURE ("remove last bare name entry from table of size 2");
794+
795+ /* Set a variable to allow the bare name to be expanded */
796+ assert0 (setenv ("UPSTART_TEST_VARIABLE", "foo", 1));
797+
798+ TEST_ALLOC_FAIL {
799+ TEST_ALLOC_SAFE {
800+ len = 0;
801+ env = nih_str_array_new (NULL);
802+
803+ ret = environ_add (&env, NULL, &len, TRUE, "FOO=BAR");
804+ TEST_NE_P (ret, NULL);
805+
806+ TEST_EQ (len, 1);
807+ TEST_ALLOC_PARENT (env[0], env);
808+ TEST_ALLOC_SIZE (env[0], 8);
809+ TEST_EQ_STR (env[0], "FOO=BAR");
810+ TEST_EQ_P (env[1], NULL);
811+
812+ ret = environ_add (&env, NULL, &len, TRUE, "UPSTART_TEST_VARIABLE");
813+ TEST_NE_P (ret, NULL);
814+
815+ TEST_EQ (len, 2);
816+ TEST_ALLOC_PARENT (env[0], env);
817+ TEST_ALLOC_SIZE (env[0], 8);
818+ TEST_EQ_STR (env[0], "FOO=BAR");
819+
820+ TEST_ALLOC_PARENT (env[1], env);
821+ TEST_ALLOC_SIZE (env[1], 8);
822+
823+ /* Should have been expanded */
824+ TEST_EQ_STR (env[1], "UPSTART_TEST_VARIABLE=foo");
825+
826+ TEST_EQ_P (env[2], NULL);
827+ }
828+
829+ /* Remove last entry added */
830+ ret = environ_remove (&env, NULL, &len, "UPSTART_TEST_VARIABLE");
831+
832+ if (test_alloc_failed) {
833+ TEST_EQ_P (ret, NULL);
834+
835+ TEST_EQ (len, 2);
836+ TEST_ALLOC_PARENT (env[0], env);
837+ TEST_ALLOC_SIZE (env[0], 8);
838+ TEST_EQ_STR (env[0], "FOO=BAR");
839+
840+ TEST_ALLOC_PARENT (env[1], env);
841+ TEST_ALLOC_SIZE (env[1], 8);
842+ TEST_EQ_STR (env[1], "UPSTART_TEST_VARIABLE=foo");
843+
844+ TEST_EQ_P (env[2], NULL);
845+
846+ nih_free (env);
847+ continue;
848+ }
849+
850+ TEST_NE_P (ret, NULL);
851+ TEST_EQ (len, 1);
852+
853+ TEST_ALLOC_PARENT (env[0], env);
854+ TEST_ALLOC_SIZE (env[0], 8);
855+ TEST_EQ_STR (env[0], "FOO=BAR");
856+
857+ TEST_EQ_P (env[1], NULL);
858+
859+ nih_free (env);
860+ }
861+
862+ assert0 (unsetenv ("UPSTART_TEST_VARIABLE"));
863 }
864
865 void
866@@ -1590,6 +2046,7 @@
867 setenv ("UPSTART_NO_SESSIONS", "1", 1);
868
869 test_add ();
870+ test_remove ();
871 test_append ();
872 test_set ();
873 test_lookup ();
874
875=== modified file 'test/test_util_common.c'
876--- test/test_util_common.c 2013-10-02 08:59:20 +0000
877+++ test/test_util_common.c 2013-10-22 10:15:27 +0000
878@@ -120,7 +120,7 @@
879 }
880
881 /* TRUE to denote that Upstart is running in user session mode
882- * (FALSE to denote it's using the users D-Bus session bus).
883+ * (FALSE to denote it's using a D-Bus session bus only).
884 */
885 int test_user_mode = FALSE;
886
887@@ -349,6 +349,16 @@
888 {
889 static char path[PATH_MAX + 1024] = { 0 };
890 int ret;
891+ int env_valid;
892+
893+ /* Sanity check calling environment */
894+ if (test_user_mode) {
895+ env_valid = getenv ("UPSTART_SESSION") ? TRUE : FALSE;
896+ } else {
897+ env_valid = getenv ("DBUS_SESSION_BUS_ADDRESS") ? TRUE : FALSE;
898+ }
899+
900+ nih_assert (env_valid);
901
902 ret = sprintf (path, "%s %s",
903 get_initctl_binary (),
904
905=== modified file 'util/initctl.c'
906--- util/initctl.c 2013-07-21 23:54:16 +0000
907+++ util/initctl.c 2013-10-22 10:15:27 +0000
908@@ -342,8 +342,7 @@
909 use_dbus = getuid () ? TRUE : FALSE;
910 if (use_dbus >= 0 && dbus_bus_type < 0)
911 dbus_bus_type = DBUS_BUS_SYSTEM;
912- }
913- else {
914+ } else {
915 if (! user_addr) {
916 nih_error ("UPSTART_SESSION isn't set in the environment. "
917 "Unable to locate the Upstart instance.");
918@@ -2827,7 +2826,7 @@
919 * Command-line options accepted for all arguments.
920 **/
921 static NihOption options[] = {
922- { 0, "session", N_("use D-Bus session bus to connect to init daemon (for testing)"),
923+ { 0, "session", N_("use D-Bus session bus to connect to init daemon"),
924 NULL, NULL, NULL, dbus_bus_type_setter },
925 { 0, "system", N_("use D-Bus system bus to connect to init daemon"),
926 NULL, NULL, NULL, dbus_bus_type_setter },
927
928=== modified file 'util/man/initctl.8'
929--- util/man/initctl.8 2013-05-31 15:41:20 +0000
930+++ util/man/initctl.8 2013-10-22 10:15:27 +0000
931@@ -47,9 +47,9 @@
932 .\"
933 .TP
934 .B \-\-session
935-Connect to
936+Connect to the
937 .BR init (8)
938-daemon using the D\-Bus session bus (for testing purposes only).
939+daemon using the D\-Bus session bus.
940 .\"
941 .TP
942 .B \-\-system
943
944=== modified file 'util/tests/test_initctl.c'
945--- util/tests/test_initctl.c 2013-09-26 16:33:07 +0000
946+++ util/tests/test_initctl.c 2013-10-22 10:15:27 +0000
947@@ -16698,6 +16698,89 @@
948 TEST_EQ (rmdir (logdir), 0);
949 }
950
951+void
952+test_session_init (void)
953+{
954+ size_t lines;
955+ pid_t dbus_pid = 0;
956+ pid_t upstart_pid = 0;
957+ nih_local char *cmd = NULL;
958+ char **output;
959+ nih_local char *dbus_session_address = NULL;
960+ nih_local char *upstart_session = NULL;
961+ char *address;
962+
963+ TEST_GROUP ("User Mode");
964+
965+ TEST_FEATURE ("ensure session init connects to D-Bus session bus");
966+
967+ /* Start a dbus-daemon */
968+ TEST_DBUS (dbus_pid);
969+
970+ /* Not required */
971+ assert0 (unsetenv ("DBUS_SYSTEM_BUS_ADDRESS"));
972+
973+ address = getenv ("DBUS_SESSION_BUS_ADDRESS");
974+ TEST_NE_P (address, NULL);
975+
976+ dbus_session_address = nih_strdup (NULL, address);
977+ TEST_NE_P (dbus_session_address, NULL);
978+
979+ /* Stop Upstart connectng to the D-Bus session bus at
980+ * startup (it will attempt to do so, but fail and try again on
981+ * SIGUSR1.
982+ */
983+ assert0 (unsetenv ("DBUS_SESSION_BUS_ADDRESS"));
984+
985+ START_UPSTART (upstart_pid, TRUE);
986+
987+ /* Save the Upstart session socket details and unset to stop
988+ * initctl finding upstart via this route.
989+ */
990+ address = getenv ("UPSTART_SESSION");
991+ TEST_NE_P (address, NULL);
992+ upstart_session = nih_strdup (NULL, address);
993+ TEST_NE_P (upstart_session, NULL);
994+ assert0 (unsetenv ("UPSTART_SESSION"));
995+
996+ /* Ensure it is not possible to query the running version via
997+ * the D-Bus session bus.
998+ */
999+ cmd = nih_sprintf (NULL, "%s --session version 2>&1", get_initctl_binary ());
1000+ TEST_NE_P (cmd, NULL);
1001+ RUN_COMMAND (NULL, cmd, &output, &lines);
1002+ TEST_EQ (lines, 1);
1003+ TEST_STR_MATCH (output[0], "initctl: Name \"com.ubuntu.Upstart\" does not exist*");
1004+
1005+ /* Re-apply in the test environment */
1006+ assert0 (setenv ("DBUS_SESSION_BUS_ADDRESS", dbus_session_address, 1));
1007+ assert0 (setenv ("UPSTART_SESSION", upstart_session, 1));
1008+
1009+ /* Set the variable in Upstarts environment too */
1010+ cmd = nih_sprintf (NULL, "%s --user set-env -g %s=%s",
1011+ get_initctl_binary (),
1012+ "DBUS_SESSION_BUS_ADDRESS",
1013+ dbus_session_address);
1014+ TEST_NE_P (cmd, NULL);
1015+ RUN_COMMAND (NULL, cmd, &output, &lines);
1016+ TEST_EQ (lines, 0);
1017+
1018+ /* Send signal to upstart requesting it reconnect to D-Bus */
1019+ assert0 (kill (upstart_pid, SIGUSR1));
1020+
1021+ /* It should now be possible to query the running version via
1022+ * the D-Bus session bus.
1023+ */
1024+ cmd = nih_sprintf (NULL, "%s --session version 2>&1", get_initctl_binary ());
1025+ TEST_NE_P (cmd, NULL);
1026+ RUN_COMMAND (NULL, cmd, &output, &lines);
1027+ TEST_EQ (lines, 1);
1028+ TEST_STR_MATCH (output[0], "init (upstart [0-9.][0-9.]*");
1029+
1030+ STOP_UPSTART (upstart_pid);
1031+ TEST_DBUS_END (dbus_pid);
1032+}
1033+
1034 int
1035 main (int argc,
1036 char *argv[])
1037@@ -16741,5 +16824,7 @@
1038 test_notify_disk_writeable ();
1039 }
1040
1041+ test_session_init ();
1042+
1043 return 0;
1044 }

Subscribers

People subscribed via source and target branches