Merge authors: James Hunt (jamesodhunt) Related merge proposals: https://code.launchpad.net/~jamesodhunt/upstart/fix-job_find/+merge/195632 proposed by: James Hunt (jamesodhunt) ------------------------------------------------------------ revno: 1599 [merge] committer: Dimitri John Ledkov <[email protected]> branch nick: trunk timestamp: Wed 2014-03-05 13:26:05 +0000 message: merge lp:~jamesodhunt/upstart/fix-job_find modified: ChangeLog init/control.c init/control.h init/job.c init/tests/test_control.c init/tests/test_job.c
-- lp:upstart https://code.launchpad.net/~upstart-devel/upstart/trunk Your team Upstart Reviewers is subscribed to branch lp:upstart. To unsubscribe from this branch go to https://code.launchpad.net/~upstart-devel/upstart/trunk/+edit-subscription
=== modified file 'ChangeLog' --- ChangeLog 2014-03-05 12:38:57 +0000 +++ ChangeLog 2014-03-05 13:26:05 +0000 @@ -1,6 +1,27 @@ 2014-03-05 James Hunt <[email protected]> * doc/states.dot: Added missing security state. + * init/control.c: + - control_set_env(): + - Check permissions before anything else. + - Explicit check on @var rather than an assert. + - control_unset_env(): Explicit check on @name rather than an assert. + - control_get_env(): Explicit check on @name. + - control_reset_env(): Check permissions before anything else. + * init/control.h: control_get_job(): + - Pass @job_name to job_find() rather than hard-coded NULL. + - Handle instance being NULL when raising NIH D-Bus error. + * init/job.c: job_find(): Handle NULL job_name. + * init/tests/test_control.c: + - Typo. + - New functions: + - test_list_env(). + - test_list_env(). + - test_get_env(). + - test_set_env(). + - test_unset_env(). + - test_reset_env(). + * init/tests/test_job.c: test_job_find(): New function. 2014-02-04 Cameron Norman <[email protected]> === modified file 'init/control.c' --- init/control.c 2013-11-05 16:15:19 +0000 +++ init/control.c 2013-11-18 16:38:32 +0000 @@ -1288,7 +1288,13 @@ nih_assert (message); nih_assert (job_details); - nih_assert (var); + + if (! control_check_permission (message)) { + nih_dbus_error_raise_printf ( + DBUS_INTERFACE_UPSTART ".Error.PermissionDenied", + _("You do not have permission to modify job environment")); + return -1; + } if (job_details[0]) { job_name = job_details[0]; @@ -1302,10 +1308,9 @@ return -1; } - if (! control_check_permission (message)) { - nih_dbus_error_raise_printf ( - DBUS_INTERFACE_UPSTART ".Error.PermissionDenied", - _("You do not have permission to modify job environment")); + if (! var || ! *var) { + nih_dbus_error_raise_printf (DBUS_ERROR_INVALID_ARGS, + _("Variable may not be empty string")); return -1; } @@ -1386,7 +1391,6 @@ nih_assert (message); nih_assert (job_details); - nih_assert (name); if (! control_check_permission (message)) { nih_dbus_error_raise_printf ( @@ -1395,6 +1399,12 @@ return -1; } + if (! name || ! *name) { + nih_dbus_error_raise_printf (DBUS_ERROR_INVALID_ARGS, + _("Variable may not be empty string")); + return -1; + } + if (job_details[0]) { job_name = job_details[0]; @@ -1490,6 +1500,12 @@ return -1; } + if (! name || ! *name) { + nih_dbus_error_raise_printf (DBUS_ERROR_INVALID_ARGS, + _("Variable may not be empty string")); + return -1; + } + if (job_details[0]) { job_name = job_details[0]; @@ -1649,6 +1665,13 @@ nih_assert (message); nih_assert (job_details); + if (! control_check_permission (message)) { + nih_dbus_error_raise_printf ( + DBUS_INTERFACE_UPSTART ".Error.PermissionDenied", + _("You do not have permission to modify job environment")); + return -1; + } + if (job_details[0]) { job_name = job_details[0]; @@ -1661,13 +1684,6 @@ return -1; } - if (! control_check_permission (message)) { - nih_dbus_error_raise_printf ( - DBUS_INTERFACE_UPSTART ".Error.PermissionDenied", - _("You do not have permission to modify job environment")); - return -1; - } - /* Verify that job name is valid */ if (job_name && ! strlen (job_name)) { nih_dbus_error_raise_printf (DBUS_ERROR_INVALID_ARGS, === modified file 'init/control.h' --- init/control.h 2013-10-25 13:49:49 +0000 +++ init/control.h 2013-11-18 16:38:32 +0000 @@ -1,6 +1,6 @@ /* upstart * - * Copyright 2009-2011 Canonical Ltd. + * Copyright © 2009-2011 Canonical Ltd. * Author: Scott James Remnant <[email protected]>. * * This program is free software; you can redistribute it and/or modify @@ -60,7 +60,7 @@ **/ #define control_get_job(session, job, job_name, instance) \ { \ - if (job_name != NULL ) { \ + if (job_name) { \ JobClass *class; \ \ class = job_class_get_registered (job_name, session); \ @@ -73,13 +73,14 @@ return -1; \ } \ \ - job = job_find (session, class, NULL, instance); \ - if (job == NULL) { \ + job = job_find (session, class, job_name, instance); \ + if (! job) { \ nih_dbus_error_raise_printf ( \ DBUS_INTERFACE_UPSTART \ ".Error.UnknownJobInstance", \ _("Unknown instance: %s of job %s"), \ - instance, job_name); \ + instance ? instance : "(null)", \ + job_name); \ return -1; \ } \ } \ === modified file 'init/job.c' --- init/job.c 2013-08-21 11:07:36 +0000 +++ init/job.c 2013-11-18 16:38:32 +0000 @@ -2350,6 +2350,9 @@ nih_assert (class || job_class); nih_assert (job_classes); + if (! job_name) + goto error; + if (! class) class = job_class_get_registered (job_class, session); === modified file 'init/tests/test_control.c' --- init/tests/test_control.c 2013-10-25 13:49:49 +0000 +++ init/tests/test_control.c 2013-11-18 16:38:32 +0000 @@ -61,9 +61,10 @@ #include "control.h" #include "errors.h" +#include "test_util_common.h" extern const char *control_server_address; - +extern int no_inherit_env; void test_server_open (void) @@ -933,7 +934,7 @@ strcat (filename, "/baz"); /* XXX: note that this will generate an error message when this - * test runs sine "/baz" does not exist as a directory. + * test runs since "/baz" does not exist as a directory. */ source3 = conf_source_new (NULL, filename, CONF_DIR); @@ -2176,6 +2177,524 @@ nih_log_priority = NIH_LOG_UNKNOWN; } +void +test_list_env (void) +{ + NihDBusMessage *message = NULL; + int ret; + nih_local char **env = NULL; + nih_local char **job_details = NULL; + nih_local char **job_details2 = NULL; + JobClass *class; + Job *job; + NihError *error; + + TEST_FUNCTION ("control_list_env"); + + nih_error_init (); + job_class_init (); + job_class_environment_init (); + + no_inherit_env = TRUE; + + message = nih_new (NULL, NihDBusMessage); + TEST_NE_P (message, NULL); + message->connection = NULL; + message->message = NULL; + + job_details = nih_str_array_new (NULL); + nih_assert (job_details); + + class = job_class_new (NULL, "foo", NULL); + TEST_NE_P (class, NULL); + + job = job_new (class, ""); + TEST_NE_P (job, NULL); + + nih_hash_add (job_classes, &class->entry); + + assert0 (job_class_environment_set ("hello=world", TRUE)); + + /************************************************************/ + TEST_FEATURE ("with empty array"); + + ret = control_list_env (NULL, message, job_details, &env); + TEST_EQ (ret, 0); + + /* Default variables that Upstart adds */ + TEST_STR_MATCH (env[0], "PATH=*"); + TEST_STR_MATCH (env[1], "TERM=*"); + + TEST_EQ_STR (env[2], "hello=world"); + TEST_EQ_P (env[3], NULL); + + /************************************************************/ + TEST_FEATURE ("with empty job class name"); + + job_details2 = nih_str_array_copy (NULL, NULL, job_details); + TEST_NE_P (job_details2, NULL); + + nih_assert (nih_str_array_add (&job_details2, + NULL, + NULL, + "")); + + ret = control_list_env (NULL, message, job_details2, &env); + TEST_LT (ret, 0); + + error = nih_error_get (); + TEST_EQ (error->number, NIH_DBUS_ERROR); + nih_free (error); + + /************************************************************/ + TEST_FEATURE ("with missing instance name"); + + /* add name */ + nih_assert (nih_str_array_add (&job_details, + NULL, + NULL, + "foo")); + + ret = control_list_env (NULL, message, job_details, &env); + TEST_LT (ret, 0); + + error = nih_error_get (); + TEST_EQ (error->number, NIH_DBUS_ERROR); + nih_free (error); + + /************************************************************/ + /* We can't wrap this in a TEST_ALLOC_FAIL() block since the + * nih_dbus_*() calls such as nih_dbus_error_raise_printf() use + * NIH_MUST() which defeats the fail macro + */ + TEST_FEATURE ("with correct job details"); + + /* add instance */ + nih_assert (nih_str_array_add (&job_details, + NULL, + NULL, + "")); + + ret = control_list_env (NULL, message, job_details, &env); + TEST_EQ (ret, 0); + + nih_free (message); +} + +void +test_get_env (void) +{ + NihDBusMessage *message = NULL; + int ret; + char *value = NULL; + nih_local char **job_details = NULL; + nih_local char **job_details2 = NULL; + JobClass *class; + Job *job; + NihError *error; + + TEST_FUNCTION ("control_get_env"); + + nih_error_init (); + job_class_init (); + job_class_environment_init (); + + no_inherit_env = TRUE; + + message = nih_new (NULL, NihDBusMessage); + TEST_NE_P (message, NULL); + message->connection = NULL; + message->message = NULL; + + job_details = nih_str_array_new (NULL); + nih_assert (job_details); + + job_details2 = nih_str_array_copy (NULL, NULL, job_details); + TEST_NE_P (job_details2, NULL); + + nih_assert (nih_str_array_add (&job_details2, + NULL, + NULL, + "")); + + class = job_class_new (NULL, "foo", NULL); + TEST_NE_P (class, NULL); + + job = job_new (class, ""); + TEST_NE_P (job, NULL); + + nih_hash_add (job_classes, &class->entry); + + assert0 (job_class_environment_set ("hello=world", TRUE)); + + /************************************************************/ + TEST_FEATURE ("with NULL variable name"); + + ret = control_get_env (NULL, message, job_details2, NULL, &value); + TEST_LT (ret, 0); + + error = nih_error_get (); + TEST_EQ (error->number, NIH_DBUS_ERROR); + nih_free (error); + + /************************************************************/ + TEST_FEATURE ("with empty variable name"); + + ret = control_get_env (NULL, message, job_details2, "", &value); + TEST_LT (ret, 0); + + error = nih_error_get (); + TEST_EQ (error->number, NIH_DBUS_ERROR); + nih_free (error); + + /************************************************************/ + TEST_FEATURE ("with empty job class name"); + + ret = control_get_env (NULL, message, job_details2, "hello", &value); + TEST_LT (ret, 0); + + error = nih_error_get (); + TEST_EQ (error->number, NIH_DBUS_ERROR); + nih_free (error); + + /************************************************************/ + TEST_FEATURE ("with missing instance name"); + + /* add name */ + nih_assert (nih_str_array_add (&job_details, + NULL, + NULL, + "foo")); + + ret = control_get_env (NULL, message, job_details, "hello", &value); + TEST_LT (ret, 0); + + error = nih_error_get (); + TEST_EQ (error->number, NIH_DBUS_ERROR); + nih_free (error); + + /************************************************************/ + TEST_FEATURE ("with correct job details"); + + /* add instance */ + nih_assert (nih_str_array_add (&job_details, + NULL, + NULL, + "")); + + ret = control_get_env (NULL, message, job_details, "hello", &value); + TEST_EQ (ret, 0); + TEST_EQ_STR (value, "world"); + + /************************************************************/ + /* tidy up */ + + nih_free (value); + nih_free (message); +} + +void +test_set_env (void) +{ + NihDBusMessage *message = NULL; + int ret; + nih_local char **job_details = NULL; + nih_local char **job_details2 = NULL; + JobClass *class; + Job *job; + NihError *error; + + TEST_FUNCTION ("control_set_env"); + + nih_error_init (); + job_class_init (); + job_class_environment_init (); + + no_inherit_env = TRUE; + + message = nih_new (NULL, NihDBusMessage); + TEST_NE_P (message, NULL); + message->connection = NULL; + message->message = NULL; + + job_details = nih_str_array_new (NULL); + nih_assert (job_details); + + job_details2 = nih_str_array_copy (NULL, NULL, job_details); + TEST_NE_P (job_details2, NULL); + + nih_assert (nih_str_array_add (&job_details2, + NULL, + NULL, + "")); + + class = job_class_new (NULL, "foo", NULL); + TEST_NE_P (class, NULL); + + job = job_new (class, ""); + TEST_NE_P (job, NULL); + + nih_hash_add (job_classes, &class->entry); + + /************************************************************/ + TEST_FEATURE ("with NULL variable name"); + + ret = control_set_env (NULL, message, job_details2, NULL, TRUE); + TEST_LT (ret, 0); + + error = nih_error_get (); + TEST_EQ (error->number, NIH_DBUS_ERROR); + nih_free (error); + + /************************************************************/ + TEST_FEATURE ("with empty variable name"); + + ret = control_set_env (NULL, message, job_details2, "", TRUE); + TEST_LT (ret, 0); + + error = nih_error_get (); + TEST_EQ (error->number, NIH_DBUS_ERROR); + nih_free (error); + + /************************************************************/ + TEST_FEATURE ("with empty job class name"); + + ret = control_set_env (NULL, message, job_details2, "hello=world", TRUE); + TEST_LT (ret, 0); + + error = nih_error_get (); + TEST_EQ (error->number, NIH_DBUS_ERROR); + nih_free (error); + + /************************************************************/ + TEST_FEATURE ("with missing instance name"); + + /* add name */ + nih_assert (nih_str_array_add (&job_details, + NULL, + NULL, + "foo")); + + ret = control_set_env (NULL, message, job_details, "hello=world", TRUE); + TEST_LT (ret, 0); + + error = nih_error_get (); + TEST_EQ (error->number, NIH_DBUS_ERROR); + nih_free (error); + + /************************************************************/ + TEST_FEATURE ("with correct job details"); + + /* add instance */ + nih_assert (nih_str_array_add (&job_details, + NULL, + NULL, + "")); + + ret = control_set_env (NULL, message, job_details, "hello=world", TRUE); + TEST_EQ (ret, 0); + + /************************************************************/ + /* tidy up */ + + nih_free (message); +} + +void +test_unset_env (void) +{ + NihDBusMessage *message = NULL; + int ret; + nih_local char **job_details = NULL; + nih_local char **job_details2 = NULL; + JobClass *class; + Job *job; + NihError *error; + + TEST_FUNCTION ("control_unset_env"); + + nih_error_init (); + job_class_init (); + job_class_environment_init (); + + no_inherit_env = TRUE; + + message = nih_new (NULL, NihDBusMessage); + TEST_NE_P (message, NULL); + message->connection = NULL; + message->message = NULL; + + job_details = nih_str_array_new (NULL); + nih_assert (job_details); + + job_details2 = nih_str_array_copy (NULL, NULL, job_details); + TEST_NE_P (job_details2, NULL); + + nih_assert (nih_str_array_add (&job_details2, + NULL, + NULL, + "")); + + class = job_class_new (NULL, "foo", NULL); + TEST_NE_P (class, NULL); + + job = job_new (class, ""); + TEST_NE_P (job, NULL); + + nih_hash_add (job_classes, &class->entry); + + assert0 (job_class_environment_set ("hello=world", TRUE)); + + /************************************************************/ + TEST_FEATURE ("with NULL variable name"); + + ret = control_unset_env (NULL, message, job_details2, NULL); + TEST_LT (ret, 0); + + error = nih_error_get (); + TEST_EQ (error->number, NIH_DBUS_ERROR); + nih_free (error); + + /************************************************************/ + TEST_FEATURE ("with empty variable name"); + + ret = control_unset_env (NULL, message, job_details2, ""); + TEST_LT (ret, 0); + + error = nih_error_get (); + TEST_EQ (error->number, NIH_DBUS_ERROR); + nih_free (error); + + /************************************************************/ + TEST_FEATURE ("with empty job class name"); + + ret = control_unset_env (NULL, message, job_details2, "hello"); + TEST_LT (ret, 0); + + error = nih_error_get (); + TEST_EQ (error->number, NIH_DBUS_ERROR); + nih_free (error); + + /************************************************************/ + TEST_FEATURE ("with missing instance name"); + + /* add name */ + nih_assert (nih_str_array_add (&job_details, + NULL, + NULL, + "foo")); + + ret = control_unset_env (NULL, message, job_details, "hello"); + TEST_LT (ret, 0); + + error = nih_error_get (); + TEST_EQ (error->number, NIH_DBUS_ERROR); + nih_free (error); + + /************************************************************/ + TEST_FEATURE ("with correct job details"); + + /* add instance */ + nih_assert (nih_str_array_add (&job_details, + NULL, + NULL, + "")); + + ret = control_unset_env (NULL, message, job_details, "hello"); + TEST_EQ (ret, 0); + + /************************************************************/ + /* tidy up */ + + nih_free (message); +} + +void +test_reset_env (void) +{ + NihDBusMessage *message = NULL; + int ret; + nih_local char **job_details = NULL; + nih_local char **job_details2 = NULL; + JobClass *class; + Job *job; + NihError *error; + + TEST_FUNCTION ("control_reset_env"); + + nih_error_init (); + job_class_init (); + job_class_environment_init (); + + no_inherit_env = TRUE; + + message = nih_new (NULL, NihDBusMessage); + TEST_NE_P (message, NULL); + message->connection = NULL; + message->message = NULL; + + job_details = nih_str_array_new (NULL); + nih_assert (job_details); + + class = job_class_new (NULL, "foo", NULL); + TEST_NE_P (class, NULL); + + job = job_new (class, ""); + TEST_NE_P (job, NULL); + + nih_hash_add (job_classes, &class->entry); + + /************************************************************/ + TEST_FEATURE ("with empty job class name"); + + job_details2 = nih_str_array_copy (NULL, NULL, job_details); + TEST_NE_P (job_details2, NULL); + + nih_assert (nih_str_array_add (&job_details2, + NULL, + NULL, + "")); + + ret = control_reset_env (NULL, message, job_details2); + TEST_LT (ret, 0); + + error = nih_error_get (); + TEST_EQ (error->number, NIH_DBUS_ERROR); + nih_free (error); + + /************************************************************/ + TEST_FEATURE ("with missing instance name"); + + /* add name */ + nih_assert (nih_str_array_add (&job_details, + NULL, + NULL, + "foo")); + + ret = control_reset_env (NULL, message, job_details); + TEST_LT (ret, 0); + + error = nih_error_get (); + TEST_EQ (error->number, NIH_DBUS_ERROR); + nih_free (error); + + /************************************************************/ + TEST_FEATURE ("with correct job details"); + + /* add instance */ + nih_assert (nih_str_array_add (&job_details, + NULL, + NULL, + "")); + + ret = control_reset_env (NULL, message, job_details); + TEST_EQ (ret, 0); + + /************************************************************/ + /* tidy up */ + + nih_free (message); +} int main (int argc, @@ -2205,5 +2724,11 @@ test_get_log_priority (); test_set_log_priority (); + test_list_env (); + test_get_env (); + test_set_env (); + test_unset_env (); + test_reset_env (); + return 0; } === modified file 'init/tests/test_job.c' --- init/tests/test_job.c 2013-08-23 12:46:42 +0000 +++ init/tests/test_job.c 2013-11-18 16:38:32 +0000 @@ -7493,6 +7493,123 @@ } } +void +test_job_find (void) +{ + ConfFile *file; + ConfSource *source; + Job *job, *ret; + JobClass *class; + Session *session; + + TEST_FUNCTION ("job_find"); + nih_error_init (); + conf_init (); + job_class_init (); + + TEST_HASH_EMPTY (job_classes); + + source = conf_source_new (NULL, "/tmp", CONF_JOB_DIR); + TEST_NE_P (source, NULL); + + file = conf_file_new (source, "/tmp/test"); + TEST_NE_P (file, NULL); + + class = file->job = job_class_new (NULL, "test", NULL); + TEST_NE_P (class, NULL); + + job = job_new (class, ""); + TEST_NE_P (job, NULL); + + TEST_HASH_EMPTY (job_classes); + TEST_TRUE (job_class_consider (class)); + TEST_HASH_NOT_EMPTY (job_classes); + + /***********************************************************/ + TEST_FEATURE ("JobClass, no job name or Session"); + + ret = job_find (NULL, class, NULL, NULL); + TEST_EQ_P (ret, NULL); + + /***********************************************************/ + TEST_FEATURE ("job class name, no job name or Session"); + + ret = job_find (NULL, NULL, "test", NULL); + TEST_EQ_P (ret, NULL); + + /***********************************************************/ + TEST_FEATURE ("JobClass+job name, no Session"); + + ret = job_find (NULL, class, NULL, ""); + TEST_EQ (ret, job); + + /***********************************************************/ + TEST_FEATURE ("job class name+job name, no Session"); + + ret = job_find (NULL, NULL, "test", ""); + TEST_EQ (ret, job); + + /***********************************************************/ + /* recreate env */ + + nih_free (conf_sources); + nih_free (job_classes); + + conf_sources = NULL; + job_classes = NULL; + + conf_init (); + job_class_init (); + + TEST_HASH_EMPTY (job_classes); + + source = conf_source_new (NULL, "/tmp", CONF_JOB_DIR); + TEST_NE_P (source, NULL); + + session = session_new (NULL, "/abc"); + TEST_NE_P (session, NULL); + session->conf_path = NIH_MUST (nih_strdup (session, "/def/ghi")); + + source->session = session; + + file = conf_file_new (source, "/tmp/test"); + TEST_NE_P (file, NULL); + + class = file->job = job_class_new (NULL, "test", session); + TEST_NE_P (class, NULL); + + job = job_new (class, ""); + TEST_NE_P (job, NULL); + + TEST_TRUE (job_class_consider (class)); + TEST_HASH_NOT_EMPTY (job_classes); + + /***********************************************************/ + TEST_FEATURE ("JobClass+Session, no job name"); + + ret = job_find (session, class, NULL, NULL); + TEST_EQ_P (ret, NULL); + + /***********************************************************/ + TEST_FEATURE ("job class name+Session, no job name"); + + ret = job_find (session, NULL, "test", NULL); + TEST_EQ_P (ret, NULL); + + /***********************************************************/ + /* clean up */ + + nih_free (conf_sources); + nih_free (job_classes); + nih_free (session); + + conf_sources = NULL; + job_classes = NULL; + + conf_init (); + job_class_init (); +} + void deserialise_ptrace_next (void) @@ -7608,6 +7725,8 @@ test_deserialise_ptrace (); + test_job_find (); + return 0; }
-- upstart-devel mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/upstart-devel
