luc14n0/glib
1
0
Fork 0
mirror of https://gitlab.gnome.org/GNOME/glib.git synced 2025-02-19 00:32:10 +01:00
Code Issues Packages Projects Releases Wiki Activity

gsubprocess: Fix a critical calling communicate() with no pipes

Browse Source 
If calling g_subprocess_communicate() on a GSubprocess with no
stdout/stderr pipe, a critical warning would be emitted from
g_memory_output_stream_steal_as_bytes(), as it would be called on a NULL
output stream.

Fix that, improve the relevant GIR annotations, and expand the unit
tests to cover it (and various other combinations of flags).

Signed-off-by: Philip Withnall <withnall@endlessm.com>

https://bugzilla.gnome.org/show_bug.cgi?id=793331
This commit is contained in:
Philip Withnall 2018-02-09 12:25:54 +00:00
parent 12a7727352
commit b4811aa15b
2 changed files with 149 additions and 58 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
gio/gsubprocess.c
View File

@ -1628,8 +1628,8 @@ g_subprocess_communicate_internal (GSubprocess *subprocess,
* @subprocess: a #GSubprocess * @subprocess: a #GSubprocess
* @stdin_buf: (nullable): data to send to the stdin of the subprocess, or %NULL * @stdin_buf: (nullable): data to send to the stdin of the subprocess, or %NULL
* @cancellable: a #GCancellable * @cancellable: a #GCancellable
* @stdout_buf: (out): data read from the subprocess stdout * @stdout_buf: (out) (nullable) (optional) (transfer full): data read from the subprocess stdout
* @stderr_buf: (out): data read from the subprocess stderr * @stderr_buf: (out) (nullable) (optional) (transfer full): data read from the subprocess stderr
* @error: a pointer to a %NULL #GError pointer, or %NULL * @error: a pointer to a %NULL #GError pointer, or %NULL
* *
* Communicate with the subprocess until it terminates, and all input * Communicate with the subprocess until it terminates, and all input
@ -1733,8 +1733,8 @@ g_subprocess_communicate_async (GSubprocess *subprocess,
* g_subprocess_communicate_finish: * g_subprocess_communicate_finish:
* @subprocess: Self * @subprocess: Self
* @result: Result * @result: Result
* @stdout_buf: (out): Return location for stdout data * @stdout_buf: (out) (nullable) (optional) (transfer full): Return location for stdout data
* @stderr_buf: (out): Return location for stderr data * @stderr_buf: (out) (nullable) (optional) (transfer full): Return location for stderr data
* @error: Error * @error: Error
* *
* Complete an invocation of g_subprocess_communicate_async(). * Complete an invocation of g_subprocess_communicate_async().
@ -1761,9 +1761,9 @@ g_subprocess_communicate_finish (GSubprocess *subprocess,
if (success) if (success)
{ {
if (stdout_buf) if (stdout_buf)
*stdout_buf = g_memory_output_stream_steal_as_bytes (state->stdout_buf); *stdout_buf = (state->stdout_buf != NULL) ? g_memory_output_stream_steal_as_bytes (state->stdout_buf) : NULL;
if (stderr_buf) if (stderr_buf)
*stderr_buf = g_memory_output_stream_steal_as_bytes (state->stderr_buf); *stderr_buf = (state->stderr_buf != NULL) ? g_memory_output_stream_steal_as_bytes (state->stderr_buf) : NULL;
} }
g_object_unref (result); g_object_unref (result);
@ -1775,8 +1775,8 @@ g_subprocess_communicate_finish (GSubprocess *subprocess,
* @subprocess: a #GSubprocess * @subprocess: a #GSubprocess
* @stdin_buf: (nullable): data to send to the stdin of the subprocess, or %NULL * @stdin_buf: (nullable): data to send to the stdin of the subprocess, or %NULL
* @cancellable: a #GCancellable * @cancellable: a #GCancellable
* @stdout_buf: (out): data read from the subprocess stdout * @stdout_buf: (out) (nullable) (optional) (transfer full): data read from the subprocess stdout
* @stderr_buf: (out): data read from the subprocess stderr * @stderr_buf: (out) (nullable) (optional) (transfer full): data read from the subprocess stderr
* @error: a pointer to a %NULL #GError pointer, or %NULL * @error: a pointer to a %NULL #GError pointer, or %NULL
* *
* Like g_subprocess_communicate(), but validates the output of the * Like g_subprocess_communicate(), but validates the output of the
@ -1882,8 +1882,8 @@ communicate_result_validate_utf8 (const char *stream_name,
* g_subprocess_communicate_utf8_finish: * g_subprocess_communicate_utf8_finish:
* @subprocess: Self * @subprocess: Self
* @result: Result * @result: Result
* @stdout_buf: (out): Return location for stdout data * @stdout_buf: (out) (nullable) (optional) (transfer full): Return location for stdout data
* @stderr_buf: (out): Return location for stderr data * @stderr_buf: (out) (nullable) (optional) (transfer full): Return location for stderr data
* @error: Error * @error: Error
* *
* Complete an invocation of g_subprocess_communicate_utf8_async(). * Complete an invocation of g_subprocess_communicate_utf8_async().

187
gio/tests/gsubprocess.c
View File

@ -695,6 +695,7 @@ test_multi_1 (void)
} }
typedef struct { typedef struct {
GSubprocessFlags flags;
gboolean is_utf8; gboolean is_utf8;
gboolean running; gboolean running;
GError *error; GError *error;
@ -706,45 +707,67 @@ on_communicate_complete (GObject *proc,
gpointer user_data) gpointer user_data)
{ {
TestAsyncCommunicateData *data = user_data; TestAsyncCommunicateData *data = user_data;
GBytes *stdout = NULL; GBytes *stdout_bytes = NULL, *stderr_bytes = NULL;
char *stdout_str = NULL; char *stdout_str = NULL, *stderr_str = NULL;
const guint8 *stdout_data; const guint8 *stdout_data;
gsize stdout_len; gsize stdout_len;
data->running = FALSE; data->running = FALSE;
if (data->is_utf8) if (data->is_utf8)
(void) g_subprocess_communicate_utf8_finish ((GSubprocess*)proc, result, (void) g_subprocess_communicate_utf8_finish ((GSubprocess*)proc, result,
&stdout_str, NULL, &data->error); &stdout_str, &stderr_str, &data->error);
else else
(void) g_subprocess_communicate_finish ((GSubprocess*)proc, result, (void) g_subprocess_communicate_finish ((GSubprocess*)proc, result,
&stdout, NULL, &data->error); &stdout_bytes, &stderr_bytes, &data->error);
if (data->error) if (data->error)
return; return;
if (!data->is_utf8) if (data->flags & G_SUBPROCESS_FLAGS_STDOUT_PIPE)
{ {
g_assert (stdout != NULL); if (data->is_utf8)
stdout_data = g_bytes_get_data (stdout, &stdout_len); {
stdout_data = (guint8*)stdout_str;
stdout_len = strlen (stdout_str);
}
else
stdout_data = g_bytes_get_data (stdout_bytes, &stdout_len);
g_assert_cmpmem (stdout_data, stdout_len, "# hello world\n", 14);
} }
else else
{ {
g_assert (stdout_str != NULL); g_assert_null (stdout_str);
stdout_data = (guint8*)stdout_str; g_assert_null (stdout_bytes);
stdout_len = strlen (stdout_str);
} }
g_assert_cmpmem (stdout_data, stdout_len, "hello world", 11); if (data->flags & G_SUBPROCESS_FLAGS_STDERR_PIPE)
if (stdout) {
g_bytes_unref (stdout); if (data->is_utf8)
g_assert_nonnull (stderr_str);
else
g_assert_nonnull (stderr_bytes);
}
else
{
g_assert_null (stderr_str);
g_assert_null (stderr_bytes);
}
g_clear_pointer (&stdout_bytes, g_bytes_unref);
g_clear_pointer (&stderr_bytes, g_bytes_unref);
g_free (stdout_str); g_free (stdout_str);
g_free (stderr_str);
} }
/* Test g_subprocess_communicate_async() works correctly with a variety of flags,
* as passed in via @test_data. */
static void static void
test_communicate_async (void) test_communicate_async (gconstpointer test_data)
{ {
GSubprocessFlags flags = GPOINTER_TO_INT (test_data);
GError *error = NULL; GError *error = NULL;
GPtrArray *args; GPtrArray *args;
TestAsyncCommunicateData data = { 0, }; TestAsyncCommunicateData data = { flags, 0, };
GSubprocess *proc; GSubprocess *proc;
GCancellable *cancellable = NULL; GCancellable *cancellable = NULL;
GBytes *input; GBytes *input;
@ -752,12 +775,14 @@ test_communicate_async (void)
args = get_test_subprocess_args ("cat", NULL); args = get_test_subprocess_args ("cat", NULL);
proc = g_subprocess_newv ((const gchar* const*)args->pdata, proc = g_subprocess_newv ((const gchar* const*)args->pdata,
G_SUBPROCESS_FLAGS_STDIN_PIPE | G_SUBPROCESS_FLAGS_STDOUT_PIPE, G_SUBPROCESS_FLAGS_STDIN_PIPE | flags,
&error); &error);
g_assert_no_error (error); g_assert_no_error (error);
g_ptr_array_free (args, TRUE); g_ptr_array_free (args, TRUE);
hellostring = "hello world"; /* Include a leading hash and trailing newline so that if this gets onto the
* test’s stdout, it doesn’t mess up TAP output. */
hellostring = "# hello world\n";
input = g_bytes_new_static (hellostring, strlen (hellostring)); input = g_bytes_new_static (hellostring, strlen (hellostring));
g_subprocess_communicate_async (proc, input, g_subprocess_communicate_async (proc, input,
@ -775,60 +800,76 @@ test_communicate_async (void)
g_object_unref (proc); g_object_unref (proc);
} }
/* Test g_subprocess_communicate() works correctly with a variety of flags,
* as passed in via @test_data. */
static void static void
test_communicate (void) test_communicate (gconstpointer test_data)
{ {
GSubprocessFlags flags = GPOINTER_TO_INT (test_data);
GError *error = NULL; GError *error = NULL;
GPtrArray *args; GPtrArray *args;
GSubprocess *proc; GSubprocess *proc;
GCancellable *cancellable = NULL; GCancellable *cancellable = NULL;
GBytes *input; GBytes *input;
const gchar *hellostring; const gchar *hellostring;
GBytes *stdout; GBytes *stdout_bytes, *stderr_bytes;
const gchar *stdout_data; const gchar *stdout_data;
gsize stdout_len; gsize stdout_len;
args = get_test_subprocess_args ("cat", NULL); args = get_test_subprocess_args ("cat", NULL);
proc = g_subprocess_newv ((const gchar* const*)args->pdata, proc = g_subprocess_newv ((const gchar* const*)args->pdata,
G_SUBPROCESS_FLAGS_STDIN_PIPE G_SUBPROCESS_FLAGS_STDIN_PIPE | flags,
| G_SUBPROCESS_FLAGS_STDOUT_PIPE
| G_SUBPROCESS_FLAGS_STDERR_MERGE,
&error); &error);
g_assert_no_error (error); g_assert_no_error (error);
g_ptr_array_free (args, TRUE); g_ptr_array_free (args, TRUE);
hellostring = "hello world"; /* Include a leading hash and trailing newline so that if this gets onto the
* test’s stdout, it doesn’t mess up TAP output. */
hellostring = "# hello world\n";
input = g_bytes_new_static (hellostring, strlen (hellostring)); input = g_bytes_new_static (hellostring, strlen (hellostring));
g_subprocess_communicate (proc, input, cancellable, &stdout, NULL, &error); g_subprocess_communicate (proc, input, cancellable, &stdout_bytes, &stderr_bytes, &error);
g_assert_no_error (error); g_assert_no_error (error);
stdout_data = g_bytes_get_data (stdout, &stdout_len);
g_assert_cmpmem (stdout_data, stdout_len, "hello world", 11); if (flags & G_SUBPROCESS_FLAGS_STDOUT_PIPE)
{
stdout_data = g_bytes_get_data (stdout_bytes, &stdout_len);
g_assert_cmpmem (stdout_data, stdout_len, "# hello world\n", 14);
}
else
g_assert_null (stdout_bytes);
if (flags & G_SUBPROCESS_FLAGS_STDERR_PIPE)
g_assert_nonnull (stderr_bytes);
else
g_assert_null (stderr_bytes);
g_bytes_unref (input); g_bytes_unref (input);
g_bytes_unref (stdout); g_clear_pointer (&stdout_bytes, g_bytes_unref);
g_clear_pointer (&stderr_bytes, g_bytes_unref);
g_object_unref (proc); g_object_unref (proc);
} }
/* Test g_subprocess_communicate_utf8_async() works correctly with a variety of
* flags, as passed in via @test_data. */
static void static void
test_communicate_utf8_async (void) test_communicate_utf8_async (gconstpointer test_data)
{ {
GSubprocessFlags flags = GPOINTER_TO_INT (test_data);
GError *error = NULL; GError *error = NULL;
GPtrArray *args; GPtrArray *args;
TestAsyncCommunicateData data = { 0, }; TestAsyncCommunicateData data = { flags, 0, };
GSubprocess *proc; GSubprocess *proc;
GCancellable *cancellable = NULL; GCancellable *cancellable = NULL;
args = get_test_subprocess_args ("cat", NULL); args = get_test_subprocess_args ("cat", NULL);
proc = g_subprocess_newv ((const gchar* const*)args->pdata, proc = g_subprocess_newv ((const gchar* const*)args->pdata,
G_SUBPROCESS_FLAGS_STDIN_PIPE | G_SUBPROCESS_FLAGS_STDOUT_PIPE, G_SUBPROCESS_FLAGS_STDIN_PIPE | flags,
&error); &error);
g_assert_no_error (error); g_assert_no_error (error);
g_ptr_array_free (args, TRUE); g_ptr_array_free (args, TRUE);
data.is_utf8 = TRUE; data.is_utf8 = TRUE;
g_subprocess_communicate_utf8_async (proc, "hello world", g_subprocess_communicate_utf8_async (proc, "# hello world\n",
cancellable, cancellable,
on_communicate_complete, on_communicate_complete,
&data); &data);
@ -842,33 +883,43 @@ test_communicate_utf8_async (void)
g_object_unref (proc); g_object_unref (proc);
} }
/* Test g_subprocess_communicate_utf8() works correctly with a variety of flags,
* as passed in via @test_data. */
static void static void
test_communicate_utf8 (void) test_communicate_utf8 (gconstpointer test_data)
{ {
GSubprocessFlags flags = GPOINTER_TO_INT (test_data);
GError *error = NULL; GError *error = NULL;
GPtrArray *args; GPtrArray *args;
GSubprocess *proc; GSubprocess *proc;
GCancellable *cancellable = NULL; GCancellable *cancellable = NULL;
const gchar *stdin_buf; const gchar *stdin_buf;
gchar *stdout_buf; gchar *stdout_buf, *stderr_buf;
args = get_test_subprocess_args ("cat", NULL); args = get_test_subprocess_args ("cat", NULL);
proc = g_subprocess_newv ((const gchar* const*)args->pdata, proc = g_subprocess_newv ((const gchar* const*)args->pdata,
G_SUBPROCESS_FLAGS_STDIN_PIPE G_SUBPROCESS_FLAGS_STDIN_PIPE | flags,
| G_SUBPROCESS_FLAGS_STDOUT_PIPE
| G_SUBPROCESS_FLAGS_STDERR_MERGE,
&error); &error);
g_assert_no_error (error); g_assert_no_error (error);
g_ptr_array_free (args, TRUE); g_ptr_array_free (args, TRUE);
stdin_buf = "hello world"; /* Include a leading hash and trailing newline so that if this gets onto the
* test’s stdout, it doesn’t mess up TAP output. */
stdin_buf = "# hello world\n";
g_subprocess_communicate_utf8 (proc, stdin_buf, cancellable, &stdout_buf, NULL, &error); g_subprocess_communicate_utf8 (proc, stdin_buf, cancellable, &stdout_buf, &stderr_buf, &error);
g_assert_no_error (error); g_assert_no_error (error);
g_assert (strcmp (stdout_buf, "hello world") == 0); if (flags & G_SUBPROCESS_FLAGS_STDOUT_PIPE)
g_assert_cmpstr (stdout_buf, ==, "# hello world\n");
else
g_assert_null (stdout_buf);
if (flags & G_SUBPROCESS_FLAGS_STDERR_PIPE)
g_assert_nonnull (stderr_buf);
else g_assert_null (stderr_buf);
g_free (stdout_buf); g_free (stdout_buf);
g_free (stderr_buf);
g_object_unref (proc); g_object_unref (proc);
} }
@ -903,15 +954,16 @@ test_communicate_nothing (void)
static void static void
test_communicate_utf8_invalid (void) test_communicate_utf8_invalid (void)
{ {
GSubprocessFlags flags = G_SUBPROCESS_FLAGS_STDOUT_PIPE;
GError *error = NULL; GError *error = NULL;
GPtrArray *args; GPtrArray *args;
TestAsyncCommunicateData data = { 0, }; TestAsyncCommunicateData data = { flags, 0, };
GSubprocess *proc; GSubprocess *proc;
GCancellable *cancellable = NULL; GCancellable *cancellable = NULL;
args = get_test_subprocess_args ("cat", NULL); args = get_test_subprocess_args ("cat", NULL);
proc = g_subprocess_newv ((const gchar* const*)args->pdata, proc = g_subprocess_newv ((const gchar* const*)args->pdata,
G_SUBPROCESS_FLAGS_STDIN_PIPE | G_SUBPROCESS_FLAGS_STDOUT_PIPE, G_SUBPROCESS_FLAGS_STDIN_PIPE | flags,
&error); &error);
g_assert_no_error (error); g_assert_no_error (error);
g_ptr_array_free (args, TRUE); g_ptr_array_free (args, TRUE);
@ -1409,6 +1461,22 @@ test_launcher_environment (void)
int int
main (int argc, char **argv) main (int argc, char **argv)
{ {
const struct
{
const gchar *subtest;
GSubprocessFlags flags;
}
flags_vectors[] =
{
{ "", G_SUBPROCESS_FLAGS_STDOUT_PIPE | G_SUBPROCESS_FLAGS_STDERR_MERGE },
{ "/no-pipes", G_SUBPROCESS_FLAGS_NONE },
{ "/separate-stderr", G_SUBPROCESS_FLAGS_STDOUT_PIPE | G_SUBPROCESS_FLAGS_STDERR_PIPE },
{ "/stdout-only", G_SUBPROCESS_FLAGS_STDOUT_PIPE },
{ "/stderr-only", G_SUBPROCESS_FLAGS_STDERR_PIPE },
{ "/stdout-silence", G_SUBPROCESS_FLAGS_STDOUT_SILENCE },
};
gsize i;
g_test_init (&argc, &argv, NULL); g_test_init (&argc, &argv, NULL);
g_test_bug_base ("https://bugzilla.gnome.org/"); g_test_bug_base ("https://bugzilla.gnome.org/");
@ -1430,12 +1498,35 @@ main (int argc, char **argv)
g_test_add_func ("/gsubprocess/cat-utf8", test_cat_utf8); g_test_add_func ("/gsubprocess/cat-utf8", test_cat_utf8);
g_test_add_func ("/gsubprocess/cat-eof", test_cat_eof); g_test_add_func ("/gsubprocess/cat-eof", test_cat_eof);
g_test_add_func ("/gsubprocess/multi1", test_multi_1); g_test_add_func ("/gsubprocess/multi1", test_multi_1);
g_test_add_func ("/gsubprocess/communicate", test_communicate);
g_test_add_func ("/gsubprocess/communicate-async", test_communicate_async); /* Add various tests for g_subprocess_communicate() with different flags. */
g_test_add_func ("/gsubprocess/communicate-utf8", test_communicate_utf8); for (i = 0; i < G_N_ELEMENTS (flags_vectors); i++)
g_test_add_func ("/gsubprocess/communicate-utf8-async", test_communicate_utf8_async); {
g_test_add_func ("/gsubprocess/communicate-utf8-invalid", test_communicate_utf8_invalid); gchar *test_path = NULL;
g_test_add_func ("/gsubprocess/communicate-nothing", test_communicate_nothing);
test_path = g_strdup_printf ("/gsubprocess/communicate%s", flags_vectors[i].subtest);
g_test_add_data_func (test_path, GINT_TO_POINTER (flags_vectors[i].flags),
test_communicate);
g_free (test_path);
test_path = g_strdup_printf ("/gsubprocess/communicate/async%s", flags_vectors[i].subtest);
g_test_add_data_func (test_path, GINT_TO_POINTER (flags_vectors[i].flags),
test_communicate_async);
g_free (test_path);
test_path = g_strdup_printf ("/gsubprocess/communicate/utf8%s", flags_vectors[i].subtest);
g_test_add_data_func (test_path, GINT_TO_POINTER (flags_vectors[i].flags),
test_communicate_utf8);
g_free (test_path);
test_path = g_strdup_printf ("/gsubprocess/communicate/utf8/async%s", flags_vectors[i].subtest);
g_test_add_data_func (test_path, GINT_TO_POINTER (flags_vectors[i].flags),
test_communicate_utf8_async);
g_free (test_path);
}
g_test_add_func ("/gsubprocess/communicate/utf8/invalid", test_communicate_utf8_invalid);
g_test_add_func ("/gsubprocess/communicate/nothing", test_communicate_nothing);
g_test_add_func ("/gsubprocess/terminate", test_terminate); g_test_add_func ("/gsubprocess/terminate", test_terminate);
g_test_add_func ("/gsubprocess/env", test_env); g_test_add_func ("/gsubprocess/env", test_env);
g_test_add_func ("/gsubprocess/env/inherit", test_env_inherit); g_test_add_func ("/gsubprocess/env/inherit", test_env_inherit);