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
=== modified file 'ChangeLog'
--- ChangeLog 2013-10-04 21:34:25 +0000
+++ ChangeLog 2013-10-22 10:15:27 +0000
@@ -1,4 +1,51 @@
12013-01-04 Steve Langasek <steve.langasek@ubuntu.com12013-10-22 James Hunt <james.hunt@ubuntu.com>
2
3 * init/control.c: control_bus_open(): Don't call nih_dbus_bus() if
4 DBUS_SESSION_BUS_ADDRESS is not set to avoid D-bus auto-launching a
5 dbus-daemon.
6 * init/environ.c: Comments.
7 * init/job_class.c:
8 - job_class_environment_init(): Superior check on whether job_environ
9 is not empty.
10 - job_class_environment_reset(): Only reset job_environ if not NULL
11 already.
12 - job_class_environment_set(): Set variable in Upstarts environment
13 too (required to allow Upstart to be aware of the D-Bus session bus
14 address when the dbus-daemon is available).
15 - job_class_environment_unset(): Unset variable from Upstarts
16 environment, but only if it is not a default variable.
17 * init/job_process.c: Formatting.
18 * init/test_control.c: Updated strings used by tests which check error
19 messages to include 'D-Bus'.
20 * init/test_environ.c:
21 - test_add(): New test:
22 - "using bare word with no corresponding variable set in environment"
23 - test_remove(): New function ("the missing test") containing 8 new tests:
24 - "remove name=value pair with empty table"
25 - "remove bare name with empty table"
26 - "remove name=value from table of size 1"
27 - "remove bare name from table of size 1"
28 - "remove first name=value entry from table of size 2"
29 - "remove first bare name entry from table of size 2"
30 - "remove last name=value entry from table of size 2"
31 - "remove last bare name entry from table of size 2"
32 * test/test_util_common.c:
33 - Formatting.
34 - get_initctl(): Added environment checks.
35 * util/initctl.c:
36 - Formatting.
37 - Removed testing comment from option text for '--session'.
38 * util/man/initctl.8: Removed testing comment for '--session'.
39 * util/tests/test_initctl.c:
40 - test_session_init(): New test that checks the Session Init now
41 connects to the D-Bus session bus.
42
432013-10-18 James Hunt <james.hunt@ubuntu.com>
44
45 * Connect to the D-Bus session bus as well as using a private socket
46 when running as a Session Init (LP: #1203595, #1235649).
47
482013-10-04 Steve Langasek <steve.langasek@ubuntu.com
249
3 * extra/upstart-local-bridge.c: use SOCKET_PATH in our event50 * extra/upstart-local-bridge.c: use SOCKET_PATH in our event
4 environment, instead of clobbering PATH. (LP: #1235480)51 environment, instead of clobbering PATH. (LP: #1235480)
552
=== modified file 'init/control.c'
--- init/control.c 2013-04-22 10:30:09 +0000
+++ init/control.c 2013-10-22 10:15:27 +0000
@@ -81,11 +81,19 @@
81 *81 *
82 * If TRUE, connect to the D-Bus session bus rather than the system bus.82 * If TRUE, connect to the D-Bus session bus rather than the system bus.
83 *83 *
84 * Used for testing.84 * Used for testing to simulate (as far as possible) a system-like init
85 * when running as a non-priv user.
85 **/86 **/
86int use_session_bus = FALSE;87int use_session_bus = FALSE;
8788
88/**89/**
90 * dbus_bus_type:
91 *
92 * Type of D-Bus bus to connect to.
93 **/
94DBusBusType dbus_bus_type;
95
96/**
89 * control_server_address:97 * control_server_address:
90 *98 *
91 * Address on which the control server may be reached.99 * Address on which the control server may be reached.
@@ -258,16 +266,35 @@
258266
259 control_init ();267 control_init ();
260268
261 control_handle_bus_type ();269 dbus_bus_type = control_get_bus_type ();
262270
263 /* Connect to the D-Bus System Bus and hook everything up into271 /* Avoid D-Bus attempting to autolaunch a dbus-daemon since:
272 *
273 * - the behaviour is undesirable: launching a dbus-daemon
274 * should be handled by a job.
275 *
276 * - if autolaunch fails (for example if DISPLAY is not set, or
277 * dbus-uuidgen has not been run (or the generated file is
278 * unreadable)), libdbus may call abort(), depending on how
279 * libdbus has been built.
280 */
281 if (dbus_bus_type == DBUS_BUS_SESSION && ! getenv ("DBUS_SESSION_BUS_ADDRESS")) {
282 nih_dbus_error_raise ("dbus session bus error",
283 "DBUS_SESSION_BUS_ADDRESS not set");
284 return -1;
285 }
286
287 /* Connect to the appropriate D-Bus bus and hook everything up into
264 * our own main loop automatically.288 * our own main loop automatically.
265 */289 */
266 conn = nih_dbus_bus (use_session_bus ? DBUS_BUS_SESSION : DBUS_BUS_SYSTEM,290 conn = nih_dbus_bus (dbus_bus_type, control_disconnected);
267 control_disconnected);
268 if (! conn)291 if (! conn)
269 return -1;292 return -1;
270293
294 nih_debug ("Connected to D-Bus %s bus",
295 dbus_bus_type == DBUS_BUS_SESSION
296 ? "session" : "system");
297
271 /* Register objects on the bus. */298 /* Register objects on the bus. */
272 control_register_all (conn);299 control_register_all (conn);
273300
@@ -345,7 +372,9 @@
345372
346 dbus_error_init (&error);373 dbus_error_init (&error);
347374
348 nih_warn (_("Disconnected from system bus"));375 nih_warn (_("Disconnected from D-Bus %s bus"),
376 dbus_bus_type == DBUS_BUS_SESSION
377 ? "session" : "system");
349378
350 control_bus = NULL;379 control_bus = NULL;
351 }380 }
@@ -817,19 +846,23 @@
817}846}
818847
819/**848/**
820 * control_handle_bus_type:849 * control_get_bus_type:
821 *850 *
822 * Determine D-Bus bus type to connect to.851 * Determine D-Bus bus type to connect to.
852 *
853 * Returns: Type of D-Bus bus to connect to.
823 **/854 **/
824void855DBusBusType
825control_handle_bus_type (void)856control_get_bus_type (void)
826{857{
827 if (getenv (USE_SESSION_BUS_ENV))858 if (getenv (USE_SESSION_BUS_ENV))
828 use_session_bus = TRUE;859 use_session_bus = TRUE;
829860
830 if (use_session_bus)861 return (use_session_bus || user_mode)
831 nih_debug ("Using session bus");862 ? DBUS_BUS_SESSION
863 : DBUS_BUS_SYSTEM;
832}864}
865
833/**866/**
834 * control_notify_disk_writeable:867 * control_notify_disk_writeable:
835 * @data: not used,868 * @data: not used,
836869
=== modified file 'init/control.h'
--- init/control.h 2013-05-08 16:21:08 +0000
+++ init/control.h 2013-10-22 10:15:27 +0000
@@ -134,7 +134,11 @@
134 const char *log_priority)134 const char *log_priority)
135 __attribute__ ((warn_unused_result));135 __attribute__ ((warn_unused_result));
136136
137void control_handle_bus_type (void);137DBusBusType control_get_bus_type (void)
138 __attribute__ ((warn_unused_result));
139
140const char *control_get_bus_name (void)
141 __attribute__ ((warn_unused_result));
138142
139void control_prepare_reexec (void);143void control_prepare_reexec (void);
140144
141145
=== modified file 'init/environ.c'
--- init/environ.c 2013-01-08 15:57:31 +0000
+++ init/environ.c 2013-10-22 10:15:27 +0000
@@ -202,6 +202,10 @@
202 for (e = *env; e && *e; e++) {202 for (e = *env; e && *e; e++) {
203 keylen = strcspn (*e, "=");203 keylen = strcspn (*e, "=");
204204
205 /* Found @str in the existing environment (either as a
206 * name=value pair, or a bare name), so don't copy it to
207 * the new environment.
208 */
205 if (! strncmp (str, *e, keylen))209 if (! strncmp (str, *e, keylen))
206 continue;210 continue;
207211
208212
=== modified file 'init/job_class.c'
--- init/job_class.c 2013-07-22 00:15:43 +0000
+++ init/job_class.c 2013-10-22 10:15:27 +0000
@@ -89,6 +89,14 @@
89NihHash *job_classes = NULL;89NihHash *job_classes = NULL;
9090
91/**91/**
92 * default_environ:
93 *
94 * Minimal set of environment variables all jobs are provided with by
95 * default.
96 */
97static char * const default_environ[] = { JOB_DEFAULT_ENVIRONMENT, NULL };
98
99/**
92 * job_environ:100 * job_environ:
93 *101 *
94 * Array of environment variables that will be set in the jobs102 * Array of environment variables that will be set in the jobs
@@ -116,8 +124,6 @@
116void124void
117job_class_environment_init (void)125job_class_environment_init (void)
118{126{
119 char * const default_environ[] = { JOB_DEFAULT_ENVIRONMENT, NULL };
120
121 if (job_environ)127 if (job_environ)
122 return;128 return;
123129
@@ -125,7 +131,7 @@
125 NIH_MUST (environ_append (&job_environ, NULL, 0, TRUE, default_environ));131 NIH_MUST (environ_append (&job_environ, NULL, 0, TRUE, default_environ));
126132
127 if (user_mode && ! no_inherit_env)133 if (user_mode && ! no_inherit_env)
128 NIH_MUST(environ_append (&job_environ, NULL, 0, TRUE, environ));134 NIH_MUST (environ_append (&job_environ, NULL, 0, TRUE, environ));
129}135}
130136
131/**137/**
@@ -138,10 +144,10 @@
138void144void
139job_class_environment_reset (void)145job_class_environment_reset (void)
140{146{
141 if (job_environ)147 if (job_environ) {
142 nih_free (job_environ);148 nih_free (job_environ);
143149 job_environ = NULL;
144 job_environ = NULL;150 }
145151
146 job_class_environment_init ();152 job_class_environment_init ();
147}153}
@@ -160,9 +166,31 @@
160int166int
161job_class_environment_set (const char *var, int replace)167job_class_environment_set (const char *var, int replace)
162{168{
169 nih_local char *name = NULL;
170 char *value;
171
163 nih_assert (var);172 nih_assert (var);
164 nih_assert (job_environ);173 nih_assert (job_environ);
165174
175 name = nih_strdup (NULL, var);
176 if (! name)
177 return -1;
178
179 value = strchr (name, '=');
180 nih_assert (value);
181 *value = '\0';
182
183 /* Jump over delimiter */
184 value++;
185
186 /* Set variable in Upstarts environment.
187 *
188 * This isn't necessary for jobs (due to job_environ), but is
189 * required to allow the Session Init to connect to the D-Bus
190 * session bus.
191 */
192 setenv (name, value, replace);
193
166 if (! environ_add (&job_environ, NULL, NULL, replace, var))194 if (! environ_add (&job_environ, NULL, NULL, replace, var))
167 return -1;195 return -1;
168196
@@ -193,9 +221,20 @@
193int221int
194job_class_environment_unset (const char *name)222job_class_environment_unset (const char *name)
195{223{
224 char * const *e;
225 int keep = FALSE;
226
196 nih_assert (name);227 nih_assert (name);
197 nih_assert (job_environ);228 nih_assert (job_environ);
198229
230 /* Determine if @name is a default variable */
231 for (e = default_environ; e && *e; e++) {
232 if (! strcmp (*e, name)) {
233 keep = TRUE;
234 break;
235 }
236 }
237
199 if (! environ_remove (&job_environ, NULL, NULL, name))238 if (! environ_remove (&job_environ, NULL, NULL, name))
200 return -1;239 return -1;
201240
@@ -211,6 +250,17 @@
211 }250 }
212 }251 }
213252
253 if (! keep) {
254 /* Remove variable from Upstarts environment, but only if it is
255 * not part of the default environment; if we did this,
256 * it would not be possible to reset the job environment
257 * since adding a variable by name would *not* be added
258 * to the job environment table since the environment
259 * lookup would fail.
260 */
261 unsetenv (name);
262 }
263
214 return 0;264 return 0;
215}265}
216266
217267
=== modified file 'init/job_process.c'
--- init/job_process.c 2013-10-03 14:43:24 +0000
+++ init/job_process.c 2013-10-22 10:15:27 +0000
@@ -278,7 +278,7 @@
278 env = NIH_MUST (nih_str_array_new (NULL));278 env = NIH_MUST (nih_str_array_new (NULL));
279279
280 if (job->env)280 if (job->env)
281 NIH_MUST(environ_append (&env, NULL, &envc, TRUE, job->env));281 NIH_MUST (environ_append (&env, NULL, &envc, TRUE, job->env));
282282
283 if (job->stop_env283 if (job->stop_env
284 && ((process == PROCESS_PRE_STOP)284 && ((process == PROCESS_PRE_STOP)
285285
=== modified file 'init/main.c'
--- init/main.c 2013-07-31 09:28:48 +0000
+++ init/main.c 2013-10-22 10:15:27 +0000
@@ -128,7 +128,7 @@
128extern int default_console;128extern int default_console;
129extern int write_state_file;129extern int write_state_file;
130extern char *log_dir;130extern char *log_dir;
131131extern DBusBusType dbus_bus_type;
132132
133/**133/**
134 * options:134 * options:
@@ -213,7 +213,8 @@
213 if (disable_job_logging)213 if (disable_job_logging)
214 nih_debug ("Job logging disabled");214 nih_debug ("Job logging disabled");
215215
216 control_handle_bus_type ();216 if (getenv (USE_SESSION_BUS_ENV))
217 use_session_bus = TRUE;
217218
218 if (! user_mode)219 if (! user_mode)
219 no_inherit_env = TRUE;220 no_inherit_env = TRUE;
@@ -933,14 +934,20 @@
933 NihSignal *signal)934 NihSignal *signal)
934{935{
935 if (! control_bus) {936 if (! control_bus) {
936 nih_info (_("Reconnecting to system bus"));937 char *dbus_bus_name;
938
939 dbus_bus_name = dbus_bus_type == DBUS_BUS_SESSION
940 ? "session" : "system";
941
942 nih_info (_("Reconnecting to D-Bus %s bus"),
943 dbus_bus_name);
937944
938 if (control_bus_open () < 0) {945 if (control_bus_open () < 0) {
939 NihError *err;946 NihError *err;
940947
941 err = nih_error_get ();948 err = nih_error_get ();
942 nih_warn ("%s: %s", _("Unable to connect to the system bus"),949 nih_warn (_("Unable to connect to the D-Bus %s bus: %s"),
943 err->message);950 dbus_bus_name, err->message);
944 nih_free (err);951 nih_free (err);
945 }952 }
946 }953 }
947954
=== modified file 'init/tests/test_control.c'
--- init/tests/test_control.c 2012-09-09 21:27:24 +0000
+++ init/tests/test_control.c 2013-10-22 10:15:27 +0000
@@ -821,7 +821,7 @@
821821
822 TEST_LIST_EMPTY (control_conns);822 TEST_LIST_EMPTY (control_conns);
823823
824 TEST_FILE_EQ (output, "test: Disconnected from system bus\n");824 TEST_FILE_EQ (output, "test: Disconnected from D-Bus system bus\n");
825 TEST_FILE_END (output);825 TEST_FILE_END (output);
826 TEST_FILE_RESET (output);826 TEST_FILE_RESET (output);
827827
@@ -879,7 +879,7 @@
879879
880 TEST_LIST_EMPTY (control_conns);880 TEST_LIST_EMPTY (control_conns);
881881
882 TEST_FILE_EQ (output, "test: Disconnected from system bus\n");882 TEST_FILE_EQ (output, "test: Disconnected from D-Bus system bus\n");
883 TEST_FILE_END (output);883 TEST_FILE_END (output);
884 TEST_FILE_RESET (output);884 TEST_FILE_RESET (output);
885885
886886
=== modified file 'init/tests/test_environ.c'
--- init/tests/test_environ.c 2011-06-06 17:05:11 +0000
+++ init/tests/test_environ.c 2013-10-22 10:15:27 +0000
@@ -572,6 +572,462 @@
572 }572 }
573573
574 unsetenv ("BAR");574 unsetenv ("BAR");
575
576 /* Check that attempting to add a variable by name fails if
577 * there is no corresponding environment variable set.
578 */
579 TEST_FEATURE ("using bare word with no corresponding variable set in environment");
580
581 /* Ensure variable not set initially */
582 TEST_EQ_P (getenv ("UPSTART_TEST_VARIABLE"), NULL);
583
584 TEST_ALLOC_FAIL {
585 TEST_ALLOC_SAFE {
586 len = 0;
587 env = nih_str_array_new (NULL);
588 assert (nih_str_array_add (&env, NULL, &len,
589 "FOO=BAR"));
590 }
591
592 ret = environ_add (&env, NULL, &len, FALSE, "UPSTART_TEST_VARIABLE");
593
594 if (test_alloc_failed) {
595 TEST_EQ_P (ret, NULL);
596
597 TEST_EQ (len, 1);
598 TEST_EQ_STR (env[0], "FOO=BAR");
599 TEST_EQ_P (env[1], NULL);
600
601 nih_free (env);
602 continue;
603 }
604
605 /* XXX: Attempting to add an unset variable results in
606 * no change to the table (it is not an error!)
607 */
608 TEST_EQ_P (ret, env);
609
610 TEST_EQ (len, 1);
611 TEST_EQ_STR (env[0], "FOO=BAR");
612 TEST_EQ_P (env[1], NULL);
613
614 nih_free (env);
615 }
616}
617
618void
619test_remove (void)
620{
621 char **env = NULL, **ret;
622 size_t len = 0;
623
624 TEST_FUNCTION ("environ_remove");
625
626 TEST_FEATURE ("remove name=value pair with empty table");
627 TEST_ALLOC_FAIL {
628 TEST_ALLOC_SAFE {
629 len = 0;
630 env = nih_str_array_new (NULL);
631 }
632
633 ret = environ_remove (&env, NULL, &len, "FOO=BAR");
634
635 if (test_alloc_failed) {
636 TEST_EQ_P (ret, NULL);
637
638 TEST_EQ (len, 0);
639 TEST_EQ_P (env[0], NULL);
640
641 nih_free (env);
642 continue;
643 }
644
645 TEST_EQ_P (ret, NULL);
646 TEST_EQ (len, 0);
647 TEST_EQ_P (env[0], NULL);
648
649 nih_free (env);
650 }
651
652 TEST_FEATURE ("remove bare name with empty table");
653 TEST_ALLOC_FAIL {
654 TEST_ALLOC_SAFE {
655 len = 0;
656 env = nih_str_array_new (NULL);
657 }
658
659 ret = environ_remove (&env, NULL, &len, "FOO");
660
661 if (test_alloc_failed) {
662 TEST_EQ_P (ret, NULL);
663
664 TEST_EQ (len, 0);
665 TEST_EQ_P (env[0], NULL);
666
667 nih_free (env);
668 continue;
669 }
670
671 TEST_EQ_P (ret, NULL);
672 TEST_EQ (len, 0);
673 TEST_EQ_P (env[0], NULL);
674
675 nih_free (env);
676 }
677
678 TEST_FEATURE ("remove name=value from table of size 1");
679 TEST_ALLOC_FAIL {
680 TEST_ALLOC_SAFE {
681 len = 0;
682 env = nih_str_array_new (NULL);
683
684 ret = environ_add (&env, NULL, &len, TRUE, "FOO=BAR");
685 TEST_NE_P (ret, NULL);
686
687 TEST_EQ (len, 1);
688 TEST_ALLOC_PARENT (env[0], env);
689 TEST_ALLOC_SIZE (env[0], 8);
690 TEST_EQ_STR (env[0], "FOO=BAR");
691 TEST_EQ_P (env[1], NULL);
692 }
693
694 ret = environ_remove (&env, NULL, &len, "FOO=BAR");
695
696 if (test_alloc_failed) {
697 TEST_EQ_P (ret, NULL);
698
699 TEST_EQ (len, 1);
700
701 TEST_ALLOC_PARENT (env[0], env);
702 TEST_ALLOC_SIZE (env[0], 8);
703
704 TEST_NE_P (env[0], NULL);
705 TEST_EQ_STR (env[0], "FOO=BAR");
706
707 TEST_EQ_P (env[1], NULL);
708
709 nih_free (env);
710 continue;
711 }
712
713 TEST_NE_P (ret, NULL);
714 TEST_EQ (len, 0);
715 TEST_EQ_P (env[0], NULL);
716
717 nih_free (env);
718 }
719
720 TEST_FEATURE ("remove bare name from table of size 1");
721 TEST_ALLOC_FAIL {
722 TEST_ALLOC_SAFE {
723 len = 0;
724 env = nih_str_array_new (NULL);
725
726 ret = environ_add (&env, NULL, &len, TRUE, "FOO=BAR");
727 TEST_NE_P (ret, NULL);
728
729 TEST_EQ (len, 1);
730 TEST_ALLOC_PARENT (env[0], env);
731 TEST_ALLOC_SIZE (env[0], 8);
732 TEST_EQ_STR (env[0], "FOO=BAR");
733 TEST_EQ_P (env[1], NULL);
734 }
735
736 ret = environ_remove (&env, NULL, &len, "FOO");
737
738 if (test_alloc_failed) {
739 TEST_EQ_P (ret, NULL);
740
741 TEST_EQ (len, 1);
742
743 TEST_ALLOC_PARENT (env[0], env);
744 TEST_ALLOC_SIZE (env[0], 8);
745
746 TEST_NE_P (env[0], NULL);
747 TEST_EQ_STR (env[0], "FOO=BAR");
748
749 TEST_EQ_P (env[1], NULL);
750
751 nih_free (env);
752 continue;
753 }
754
755 TEST_NE_P (ret, NULL);
756 TEST_EQ (len, 0);
757 TEST_EQ_P (env[0], NULL);
758
759 nih_free (env);
760 }
761
762 TEST_FEATURE ("remove first name=value entry from table of size 2");
763 TEST_ALLOC_FAIL {
764 TEST_ALLOC_SAFE {
765 len = 0;
766 env = nih_str_array_new (NULL);
767
768 ret = environ_add (&env, NULL, &len, TRUE, "FOO=BAR");
769 TEST_NE_P (ret, NULL);
770
771 TEST_EQ (len, 1);
772 TEST_ALLOC_PARENT (env[0], env);
773 TEST_ALLOC_SIZE (env[0], 8);
774 TEST_EQ_STR (env[0], "FOO=BAR");
775 TEST_EQ_P (env[1], NULL);
776
777 ret = environ_add (&env, NULL, &len, TRUE, "BAZ=QUX");
778 TEST_NE_P (ret, NULL);
779
780 TEST_EQ (len, 2);
781 TEST_ALLOC_PARENT (env[0], env);
782 TEST_ALLOC_SIZE (env[0], 8);
783 TEST_EQ_STR (env[0], "FOO=BAR");
784
785 TEST_ALLOC_PARENT (env[1], env);
786 TEST_ALLOC_SIZE (env[1], 8);
787 TEST_EQ_STR (env[1], "BAZ=QUX");
788
789 TEST_EQ_P (env[2], NULL);
790 }
791
792 /* Remove first entry added */
793 ret = environ_remove (&env, NULL, &len, "FOO=BAR");
794
795 if (test_alloc_failed) {
796 TEST_EQ_P (ret, NULL);
797
798 TEST_EQ (len, 2);
799 TEST_ALLOC_PARENT (env[0], env);
800 TEST_ALLOC_SIZE (env[0], 8);
801 TEST_EQ_STR (env[0], "FOO=BAR");
802
803 TEST_ALLOC_PARENT (env[1], env);
804 TEST_ALLOC_SIZE (env[1], 8);
805 TEST_EQ_STR (env[1], "BAZ=QUX");
806
807 TEST_EQ_P (env[2], NULL);
808
809 nih_free (env);
810 continue;
811 }
812
813 TEST_NE_P (ret, NULL);
814 TEST_EQ (len, 1);
815
816 TEST_ALLOC_PARENT (env[0], env);
817 TEST_ALLOC_SIZE (env[0], 8);
818 TEST_EQ_STR (env[0], "BAZ=QUX");
819
820 TEST_EQ_P (env[1], NULL);
821
822 nih_free (env);
823 }
824
825 TEST_FEATURE ("remove first bare name entry from table of size 2");
826
827 /* Set a variable to allow the bare name to be expanded */
828 assert0 (setenv ("UPSTART_TEST_VARIABLE", "foo", 1));
829
830 TEST_ALLOC_FAIL {
831 TEST_ALLOC_SAFE {
832 len = 0;
833 env = nih_str_array_new (NULL);
834
835 ret = environ_add (&env, NULL, &len, TRUE, "UPSTART_TEST_VARIABLE");
836 TEST_NE_P (ret, NULL);
837
838 TEST_EQ (len, 1);
839 TEST_ALLOC_PARENT (env[0], env);
840 TEST_ALLOC_SIZE (env[0], 8);
841
842 /* Should have been expanded */
843 TEST_EQ_STR (env[0], "UPSTART_TEST_VARIABLE=foo");
844
845 TEST_EQ_P (env[1], NULL);
846
847 ret = environ_add (&env, NULL, &len, TRUE, "BAZ=QUX");
848 TEST_NE_P (ret, NULL);
849
850 TEST_EQ (len, 2);
851
852 TEST_ALLOC_PARENT (env[0], env);
853 TEST_ALLOC_SIZE (env[0], 8);
854 TEST_EQ_STR (env[0], "UPSTART_TEST_VARIABLE=foo");
855
856 TEST_ALLOC_PARENT (env[1], env);
857 TEST_ALLOC_SIZE (env[1], 8);
858 TEST_EQ_STR (env[1], "BAZ=QUX");
859
860 TEST_EQ_P (env[2], NULL);
861 }
862
863 /* Remove first entry added */
864 ret = environ_remove (&env, NULL, &len, "UPSTART_TEST_VARIABLE");
865
866 if (test_alloc_failed) {
867 TEST_EQ_P (ret, NULL);
868
869 TEST_EQ (len, 2);
870 TEST_ALLOC_PARENT (env[0], env);
871 TEST_ALLOC_SIZE (env[0], 8);
872 TEST_EQ_STR (env[0], "UPSTART_TEST_VARIABLE=foo");
873
874 TEST_ALLOC_PARENT (env[1], env);
875 TEST_ALLOC_SIZE (env[1], 8);
876 TEST_EQ_STR (env[1], "BAZ=QUX");
877
878 TEST_EQ_P (env[2], NULL);
879
880 nih_free (env);
881 continue;
882 }
883
884 TEST_NE_P (ret, NULL);
885 TEST_EQ (len, 1);
886
887 TEST_ALLOC_PARENT (env[0], env);
888 TEST_ALLOC_SIZE (env[0], 8);
889 TEST_EQ_STR (env[0], "BAZ=QUX");
890
891 TEST_EQ_P (env[1], NULL);
892
893 nih_free (env);
894 }
895
896 assert0 (unsetenv ("UPSTART_TEST_VARIABLE"));
897
898 TEST_FEATURE ("remove last name=value entry from table of size 2");
899 TEST_ALLOC_FAIL {
900 TEST_ALLOC_SAFE {
901 len = 0;
902 env = nih_str_array_new (NULL);
903
904 ret = environ_add (&env, NULL, &len, TRUE, "FOO=BAR");
905 TEST_NE_P (ret, NULL);
906
907 TEST_EQ (len, 1);
908 TEST_ALLOC_PARENT (env[0], env);
909 TEST_ALLOC_SIZE (env[0], 8);
910 TEST_EQ_STR (env[0], "FOO=BAR");
911 TEST_EQ_P (env[1], NULL);
912
913 ret = environ_add (&env, NULL, &len, TRUE, "BAZ=QUX");
914 TEST_NE_P (ret, NULL);
915
916 TEST_EQ (len, 2);
917 TEST_ALLOC_PARENT (env[0], env);
918 TEST_ALLOC_SIZE (env[0], 8);
919 TEST_EQ_STR (env[0], "FOO=BAR");
920
921 TEST_ALLOC_PARENT (env[1], env);
922 TEST_ALLOC_SIZE (env[1], 8);
923 TEST_EQ_STR (env[1], "BAZ=QUX");
924
925 TEST_EQ_P (env[2], NULL);
926 }
927
928 /* Remove last entry added */
929 ret = environ_remove (&env, NULL, &len, "BAZ=QUX");
930
931 if (test_alloc_failed) {
932 TEST_EQ_P (ret, NULL);
933
934 TEST_EQ (len, 2);
935 TEST_ALLOC_PARENT (env[0], env);
936 TEST_ALLOC_SIZE (env[0], 8);
937 TEST_EQ_STR (env[0], "FOO=BAR");
938
939 TEST_ALLOC_PARENT (env[1], env);
940 TEST_ALLOC_SIZE (env[1], 8);
941 TEST_EQ_STR (env[1], "BAZ=QUX");
942
943 TEST_EQ_P (env[2], NULL);
944
945 nih_free (env);
946 continue;
947 }
948
949 TEST_NE_P (ret, NULL);
950 TEST_EQ (len, 1);
951
952 TEST_ALLOC_PARENT (env[0], env);
953 TEST_ALLOC_SIZE (env[0], 8);
954 TEST_EQ_STR (env[0], "FOO=BAR");
955
956 TEST_EQ_P (env[1], NULL);
957
958 nih_free (env);
959 }
960
961 TEST_FEATURE ("remove last bare name entry from table of size 2");
962
963 /* Set a variable to allow the bare name to be expanded */
964 assert0 (setenv ("UPSTART_TEST_VARIABLE", "foo", 1));
965
966 TEST_ALLOC_FAIL {
967 TEST_ALLOC_SAFE {
968 len = 0;
969 env = nih_str_array_new (NULL);
970
971 ret = environ_add (&env, NULL, &len, TRUE, "FOO=BAR");
972 TEST_NE_P (ret, NULL);
973
974 TEST_EQ (len, 1);
975 TEST_ALLOC_PARENT (env[0], env);
976 TEST_ALLOC_SIZE (env[0], 8);
977 TEST_EQ_STR (env[0], "FOO=BAR");
978 TEST_EQ_P (env[1], NULL);
979
980 ret = environ_add (&env, NULL, &len, TRUE, "UPSTART_TEST_VARIABLE");
981 TEST_NE_P (ret, NULL);
982
983 TEST_EQ (len, 2);
984 TEST_ALLOC_PARENT (env[0], env);
985 TEST_ALLOC_SIZE (env[0], 8);
986 TEST_EQ_STR (env[0], "FOO=BAR");
987
988 TEST_ALLOC_PARENT (env[1], env);
989 TEST_ALLOC_SIZE (env[1], 8);
990
991 /* Should have been expanded */
992 TEST_EQ_STR (env[1], "UPSTART_TEST_VARIABLE=foo");
993
994 TEST_EQ_P (env[2], NULL);
995 }
996
997 /* Remove last entry added */
998 ret = environ_remove (&env, NULL, &len, "UPSTART_TEST_VARIABLE");
999
1000 if (test_alloc_failed) {
1001 TEST_EQ_P (ret, NULL);
1002
1003 TEST_EQ (len, 2);
1004 TEST_ALLOC_PARENT (env[0], env);
1005 TEST_ALLOC_SIZE (env[0], 8);
1006 TEST_EQ_STR (env[0], "FOO=BAR");
1007
1008 TEST_ALLOC_PARENT (env[1], env);
1009 TEST_ALLOC_SIZE (env[1], 8);
1010 TEST_EQ_STR (env[1], "UPSTART_TEST_VARIABLE=foo");
1011
1012 TEST_EQ_P (env[2], NULL);
1013
1014 nih_free (env);
1015 continue;
1016 }
1017
1018 TEST_NE_P (ret, NULL);
1019 TEST_EQ (len, 1);
1020
1021 TEST_ALLOC_PARENT (env[0], env);
1022 TEST_ALLOC_SIZE (env[0], 8);
1023 TEST_EQ_STR (env[0], "FOO=BAR");
1024
1025 TEST_EQ_P (env[1], NULL);
1026
1027 nih_free (env);
1028 }
1029
1030 assert0 (unsetenv ("UPSTART_TEST_VARIABLE"));
575}1031}
5761032
577void1033void
@@ -1590,6 +2046,7 @@
1590 setenv ("UPSTART_NO_SESSIONS", "1", 1);2046 setenv ("UPSTART_NO_SESSIONS", "1", 1);
15912047
1592 test_add ();2048 test_add ();
2049 test_remove ();
1593 test_append ();2050 test_append ();
1594 test_set ();2051 test_set ();
1595 test_lookup ();2052 test_lookup ();
15962053
=== modified file 'test/test_util_common.c'
--- test/test_util_common.c 2013-10-02 08:59:20 +0000
+++ test/test_util_common.c 2013-10-22 10:15:27 +0000
@@ -120,7 +120,7 @@
120}120}
121121
122/* TRUE to denote that Upstart is running in user session mode122/* TRUE to denote that Upstart is running in user session mode
123 * (FALSE to denote it's using the users D-Bus session bus).123 * (FALSE to denote it's using a D-Bus session bus only).
124 */124 */
125int test_user_mode = FALSE;125int test_user_mode = FALSE;
126126
@@ -349,6 +349,16 @@
349{349{
350 static char path[PATH_MAX + 1024] = { 0 };350 static char path[PATH_MAX + 1024] = { 0 };
351 int ret;351 int ret;
352 int env_valid;
353
354 /* Sanity check calling environment */
355 if (test_user_mode) {
356 env_valid = getenv ("UPSTART_SESSION") ? TRUE : FALSE;
357 } else {
358 env_valid = getenv ("DBUS_SESSION_BUS_ADDRESS") ? TRUE : FALSE;
359 }
360
361 nih_assert (env_valid);
352362
353 ret = sprintf (path, "%s %s",363 ret = sprintf (path, "%s %s",
354 get_initctl_binary (),364 get_initctl_binary (),
355365
=== modified file 'util/initctl.c'
--- util/initctl.c 2013-07-21 23:54:16 +0000
+++ util/initctl.c 2013-10-22 10:15:27 +0000
@@ -342,8 +342,7 @@
342 use_dbus = getuid () ? TRUE : FALSE;342 use_dbus = getuid () ? TRUE : FALSE;
343 if (use_dbus >= 0 && dbus_bus_type < 0)343 if (use_dbus >= 0 && dbus_bus_type < 0)
344 dbus_bus_type = DBUS_BUS_SYSTEM;344 dbus_bus_type = DBUS_BUS_SYSTEM;
345 }345 } else {
346 else {
347 if (! user_addr) {346 if (! user_addr) {
348 nih_error ("UPSTART_SESSION isn't set in the environment. "347 nih_error ("UPSTART_SESSION isn't set in the environment. "
349 "Unable to locate the Upstart instance.");348 "Unable to locate the Upstart instance.");
@@ -2827,7 +2826,7 @@
2827 * Command-line options accepted for all arguments.2826 * Command-line options accepted for all arguments.
2828 **/2827 **/
2829static NihOption options[] = {2828static NihOption options[] = {
2830 { 0, "session", N_("use D-Bus session bus to connect to init daemon (for testing)"),2829 { 0, "session", N_("use D-Bus session bus to connect to init daemon"),
2831 NULL, NULL, NULL, dbus_bus_type_setter },2830 NULL, NULL, NULL, dbus_bus_type_setter },
2832 { 0, "system", N_("use D-Bus system bus to connect to init daemon"),2831 { 0, "system", N_("use D-Bus system bus to connect to init daemon"),
2833 NULL, NULL, NULL, dbus_bus_type_setter },2832 NULL, NULL, NULL, dbus_bus_type_setter },
28342833
=== modified file 'util/man/initctl.8'
--- util/man/initctl.8 2013-05-31 15:41:20 +0000
+++ util/man/initctl.8 2013-10-22 10:15:27 +0000
@@ -47,9 +47,9 @@
47.\"47.\"
48.TP48.TP
49.B \-\-session49.B \-\-session
50Connect to50Connect to the
51.BR init (8)51.BR init (8)
52daemon using the D\-Bus session bus (for testing purposes only).52daemon using the D\-Bus session bus.
53.\"53.\"
54.TP54.TP
55.B \-\-system55.B \-\-system
5656
=== modified file 'util/tests/test_initctl.c'
--- util/tests/test_initctl.c 2013-09-26 16:33:07 +0000
+++ util/tests/test_initctl.c 2013-10-22 10:15:27 +0000
@@ -16698,6 +16698,89 @@
16698 TEST_EQ (rmdir (logdir), 0);16698 TEST_EQ (rmdir (logdir), 0);
16699}16699}
1670016700
16701void
16702test_session_init (void)
16703{
16704 size_t lines;
16705 pid_t dbus_pid = 0;
16706 pid_t upstart_pid = 0;
16707 nih_local char *cmd = NULL;
16708 char **output;
16709 nih_local char *dbus_session_address = NULL;
16710 nih_local char *upstart_session = NULL;
16711 char *address;
16712
16713 TEST_GROUP ("User Mode");
16714
16715 TEST_FEATURE ("ensure session init connects to D-Bus session bus");
16716
16717 /* Start a dbus-daemon */
16718 TEST_DBUS (dbus_pid);
16719
16720 /* Not required */
16721 assert0 (unsetenv ("DBUS_SYSTEM_BUS_ADDRESS"));
16722
16723 address = getenv ("DBUS_SESSION_BUS_ADDRESS");
16724 TEST_NE_P (address, NULL);
16725
16726 dbus_session_address = nih_strdup (NULL, address);
16727 TEST_NE_P (dbus_session_address, NULL);
16728
16729 /* Stop Upstart connectng to the D-Bus session bus at
16730 * startup (it will attempt to do so, but fail and try again on
16731 * SIGUSR1.
16732 */
16733 assert0 (unsetenv ("DBUS_SESSION_BUS_ADDRESS"));
16734
16735 START_UPSTART (upstart_pid, TRUE);
16736
16737 /* Save the Upstart session socket details and unset to stop
16738 * initctl finding upstart via this route.
16739 */
16740 address = getenv ("UPSTART_SESSION");
16741 TEST_NE_P (address, NULL);
16742 upstart_session = nih_strdup (NULL, address);
16743 TEST_NE_P (upstart_session, NULL);
16744 assert0 (unsetenv ("UPSTART_SESSION"));
16745
16746 /* Ensure it is not possible to query the running version via
16747 * the D-Bus session bus.
16748 */
16749 cmd = nih_sprintf (NULL, "%s --session version 2>&1", get_initctl_binary ());
16750 TEST_NE_P (cmd, NULL);
16751 RUN_COMMAND (NULL, cmd, &output, &lines);
16752 TEST_EQ (lines, 1);
16753 TEST_STR_MATCH (output[0], "initctl: Name \"com.ubuntu.Upstart\" does not exist*");
16754
16755 /* Re-apply in the test environment */
16756 assert0 (setenv ("DBUS_SESSION_BUS_ADDRESS", dbus_session_address, 1));
16757 assert0 (setenv ("UPSTART_SESSION", upstart_session, 1));
16758
16759 /* Set the variable in Upstarts environment too */
16760 cmd = nih_sprintf (NULL, "%s --user set-env -g %s=%s",
16761 get_initctl_binary (),
16762 "DBUS_SESSION_BUS_ADDRESS",
16763 dbus_session_address);
16764 TEST_NE_P (cmd, NULL);
16765 RUN_COMMAND (NULL, cmd, &output, &lines);
16766 TEST_EQ (lines, 0);
16767
16768 /* Send signal to upstart requesting it reconnect to D-Bus */
16769 assert0 (kill (upstart_pid, SIGUSR1));
16770
16771 /* It should now be possible to query the running version via
16772 * the D-Bus session bus.
16773 */
16774 cmd = nih_sprintf (NULL, "%s --session version 2>&1", get_initctl_binary ());
16775 TEST_NE_P (cmd, NULL);
16776 RUN_COMMAND (NULL, cmd, &output, &lines);
16777 TEST_EQ (lines, 1);
16778 TEST_STR_MATCH (output[0], "init (upstart [0-9.][0-9.]*");
16779
16780 STOP_UPSTART (upstart_pid);
16781 TEST_DBUS_END (dbus_pid);
16782}
16783
16701int16784int
16702main (int argc,16785main (int argc,
16703 char *argv[])16786 char *argv[])
@@ -16741,5 +16824,7 @@
16741 test_notify_disk_writeable ();16824 test_notify_disk_writeable ();
16742 }16825 }
1674316826
16827 test_session_init ();
16828
16744 return 0;16829 return 0;
16745}16830}

Subscribers

People subscribed via source and target branches