3 files changed, 162 insertions, 140 deletions

diff --git a/clients/common/nm-polkit-listener.c b/clients/common/nm-polkit-listener.c
index ae40206ca7..7691ff4e06 100644
--- a/clients/common/nm-polkit-listener.c
+++ b/clients/common/nm-polkit-listener.c
@@ -27,6 +27,7 @@
 #include <fcntl.h>
 
 #include "nm-glib-aux/nm-dbus-aux.h"
+#include "nm-glib-aux/nm-str-buf.h"
 #include "nm-glib-aux/nm-secret-utils.h"
 #include "nm-glib-aux/nm-io-utils.h"
 #include "nm-libnm-core-intern/nm-auth-subject.h"
@@ -84,19 +85,22 @@ typedef struct {
 	CList request_lst;
 
 	NMPolkitListener *listener;
+	GSource *child_stdout_watch_source;
+	GSource *child_stdin_watch_source;
+	GDBusMethodInvocation *dbus_invocation;
 	char *action_id;
 	char *message;
 	char *username;
 	char *cookie;
-	GString *in_buffer;
-	GString *out_buffer;
-	size_t out_buffer_offset;
+	NMStrBuf in_buffer;
+	NMStrBuf out_buffer;
+	gsize out_buffer_offset;
 
 	int child_stdout;
 	int child_stdin;
-	GSource *child_stdout_watch_source;
-	GSource *child_stdin_watch_source;
-	GDBusMethodInvocation *dbus_invocation;
+
+	bool request_any_response:1;
+	bool request_is_completed:1;
 } AuthRequest;
 
 static const GDBusInterfaceInfo interface_info = NM_DEFINE_GDBUS_INTERFACE_INFO_INIT (
@@ -123,10 +127,18 @@ static const GDBusInterfaceInfo interface_info = NM_DEFINE_GDBUS_INTERFACE_INFO_
 );
 
 static void
-remove_request (AuthRequest *request)
+auth_request_complete (AuthRequest *request, gboolean success)
 {
 	c_list_unlink (&request->request_lst);
 
+	if (success)
+		g_dbus_method_invocation_return_value (request->dbus_invocation, NULL);
+	else {
+		g_dbus_method_invocation_return_dbus_error (request->dbus_invocation,
+		                                            POLKIT_DBUS_ERROR_FAILED,
+		                                            "");
+	}
+
 	nm_clear_g_free (&request->action_id);
 	nm_clear_g_free (&request->message);
 	nm_clear_g_free (&request->username);
@@ -134,10 +146,8 @@ remove_request (AuthRequest *request)
 	nm_clear_g_source_inst (&request->child_stdout_watch_source);
 	nm_clear_g_source_inst (&request->child_stdin_watch_source);
 
-	nm_explicit_bzero (request->out_buffer->str,
-	                   request->out_buffer->len);
-	g_string_free (request->out_buffer, TRUE);
-	g_string_free (request->in_buffer, TRUE);
+	nm_str_buf_destroy (&request->in_buffer);
+	nm_str_buf_destroy (&request->out_buffer);
 
 	if (request->child_stdout != -1) {
 		nm_close (request->child_stdout);
@@ -149,7 +159,7 @@ remove_request (AuthRequest *request)
 		request->child_stdin = -1;
 	}
 
-	g_slice_free (AuthRequest, request);
+	nm_g_slice_free (request);
 }
 
 static gboolean
@@ -384,60 +394,42 @@ retrieve_session_id (NMPolkitListener *self)
 	                        self);
 }
 
-static void
-complete_authentication (AuthRequest *request,
-                         gboolean result)
-{
-	if (result) {
-		g_dbus_method_invocation_return_value (request->dbus_invocation, NULL);
-	} else {
-		g_dbus_method_invocation_return_dbus_error (request->dbus_invocation,
-		                                            "org.freedesktop.PolicyKit1.Error.Failed",
-		                                            "");
-	}
-	remove_request (request);
-}
-
 static gboolean
 io_watch_can_write (int fd,
                     GIOCondition condition,
                     gpointer user_data)
 {
 	AuthRequest *request = user_data;
-	ssize_t n_written;
-	gboolean done = FALSE;
+	gssize n_written;
 
-	if (condition & G_IO_HUP ||
-	    condition & G_IO_ERR) {
-		done = TRUE;
+	if (NM_FLAGS_ANY (condition, (G_IO_HUP | G_IO_ERR)))
 		goto done;
-	}
 
 	n_written = write (request->child_stdin,
-	                   &request->out_buffer->str[request->out_buffer_offset],
-	                   request->out_buffer->len - request->out_buffer_offset);
+	                   &((nm_str_buf_get_str_unsafe (&request->out_buffer))[request->out_buffer_offset]),
+	                   request->out_buffer.len - request->out_buffer_offset);
 
-	if (n_written < 0 && errno != EAGAIN) {
-		done = TRUE;
+	if (   n_written < 0
+	    && errno != EAGAIN)
 		goto done;
-	}
 
 	if (n_written > 0) {
-		if ((size_t) n_written == (request->out_buffer->len - request->out_buffer_offset)) {
-			done = TRUE;
+		if ((gsize) n_written >= (request->out_buffer.len - request->out_buffer_offset)) {
+			nm_assert ((gsize) n_written == (request->out_buffer.len - request->out_buffer_offset));
 			goto done;
 		}
-		request->out_buffer_offset += n_written;
+		request->out_buffer_offset += (gsize) n_written;
 	}
 
+	return G_SOURCE_CONTINUE;
+
 done:
-	if (done) {
-		nm_explicit_bzero (request->out_buffer->str,
-		                   request->out_buffer->len);
-		g_string_set_size (request->out_buffer, 0);
-		request->out_buffer_offset = 0;
-		nm_clear_g_source_inst (&request->child_stdin_watch_source);
-	}
+	nm_str_buf_set_size (&request->out_buffer, 0, TRUE, FALSE);
+	request->out_buffer_offset = 0;
+	nm_clear_g_source_inst (&request->child_stdin_watch_source);
+
+	if (request->request_is_completed)
+		auth_request_complete (request, TRUE);
 
 	return G_SOURCE_CONTINUE;
 }
@@ -447,11 +439,11 @@ queue_string_to_helper (AuthRequest *request, const char *response)
 {
 	g_return_if_fail (response);
 
-	g_string_append (request->out_buffer, response);
+	if (!nm_str_buf_is_initalized (&request->out_buffer))
+		nm_str_buf_init (&request->out_buffer, strlen (response) + 2u,  TRUE);
 
-	if (   request->out_buffer->len == 0
-	    || request->out_buffer->str[request->out_buffer->len - 1] != '\n')
-		g_string_append_c (request->out_buffer, '\n');
+	nm_str_buf_append (&request->out_buffer, response);
+	nm_str_buf_ensure_trailing_c (&request->out_buffer, '\n');
 
 	if (!request->child_stdin_watch_source) {
 		request->child_stdin_watch_source = nm_g_unix_fd_source_new (request->child_stdin,
@@ -471,74 +463,87 @@ io_watch_have_data (int fd,
                     gpointer user_data)
 {
 	AuthRequest *request = user_data;
-	gs_free char *unescaped = NULL;
-	char *response = NULL;
-	char* line_terminator = 0;
-	gboolean auth_result = FALSE;
-	gboolean complete_auth = FALSE;
-	ssize_t n_read;
-
-	if (condition & G_IO_HUP ||
-	    condition & G_IO_ERR) {
-		complete_auth = TRUE;
-		auth_result = FALSE;
-		goto out;
-	}
-
-	n_read = nm_utils_fd_read (fd, request->in_buffer);
-
-	if (n_read == -EAGAIN) {
-		return G_SOURCE_CONTINUE;
-	}
+	gboolean auth_result;
+	gssize n_read;
+
+	if (NM_FLAGS_ANY (condition, G_IO_HUP | G_IO_ERR))
+		n_read = -EIO;
+	else
+		n_read = nm_utils_fd_read (fd, &request->in_buffer);
+
+	if (n_read <= 0) {
+		if (n_read == -EAGAIN) {
+			/* wait longer. */
+			return G_SOURCE_CONTINUE;
+		}
 
-	if (n_read < 0) {
-		complete_auth = TRUE;
-		auth_result = FALSE;
+		/* Either an error or EOF happened. The data we parsed so far was not relevant.
+		 * Regardless of what we still have unprocessed in the receive buffers, we are done.
+		 *
+		 * We would expect that the other side completed with SUCCESS or FAILURE. Apparently
+		 * it didn't. If we had any good request, we assume success. */
+		auth_result = request->request_any_response;
 		goto out;
 	}
 
-	line_terminator = strchr (request->in_buffer->str, '\n');
-	if (!line_terminator) {
-		return G_SOURCE_CONTINUE;
-	}
-	*line_terminator = '\0';
-
-	unescaped = g_strcompress (request->in_buffer->str);
+	while (TRUE) {
+		char *line_terminator;
+		const char *line;
 
-	if (NM_STR_HAS_PREFIX (unescaped, "PAM_PROMPT_ECHO")) {
-		/* FIXME(cli-async): emit signal and wait for response (blocking) */
-		g_signal_emit (request->listener,
-		               signals[REQUEST_SYNC],
-		               0,
-		               request->action_id,
-		               request->message,
-		               request->username,
-		               &response);
-
-		if (response) {
-			queue_string_to_helper (request, response);
-			nm_free_secret (response);
+		line = nm_str_buf_get_str (&request->in_buffer);
+		line_terminator = (char *) strchr (line, '\n');
+		if (!line_terminator) {
+			/* We don't have a complete line. Wait longer. */
+			return G_SOURCE_CONTINUE;
+		}
+		line_terminator[0] = '\0';
+
+		if (   NM_STR_HAS_PREFIX (line, "PAM_PROMPT_ECHO_OFF ")
+		    || NM_STR_HAS_PREFIX (line, "PAM_PROMPT_ECHO_ON ")) {
+			nm_auto_free_secret char *response = NULL;
+
+			/* FIXME(cli-async): emit signal and wait for response (blocking) */
+			g_signal_emit (request->listener,
+			               signals[REQUEST_SYNC],
+			               0,
+			               request->action_id,
+			               request->message,
+			               request->username,
+			               &response);
+
+			if (response) {
+				queue_string_to_helper (request, response);
+				request->request_any_response = TRUE;
+				goto erase_line;
+			}
+			auth_result = FALSE;
+		} else if (nm_streq (line, "SUCCESS"))
+			auth_result = TRUE;
+		else if (nm_streq (line, "FAILURE"))
+			auth_result = FALSE;
+		else if (   NM_STR_HAS_PREFIX (line, "PAM_ERROR_MSG ")
+		         || NM_STR_HAS_PREFIX (line, "PAM_TEXT_INFO ")) {
+			/* ignore. */
+			goto erase_line;
 		} else {
-			complete_auth = TRUE;
+			/* unknown command. Fail. */
 			auth_result = FALSE;
 		}
-	} else if (NM_STR_HAS_PREFIX (unescaped, "SUCCESS")) {
-		complete_auth = TRUE;
-		auth_result = TRUE;
-	} else if (NM_STR_HAS_PREFIX (unescaped, "FAILURE")) {
-		complete_auth = TRUE;
-		auth_result = FALSE;
-	} else {
-		complete_auth = TRUE;
-		auth_result = FALSE;
+
+		break;
+erase_line:
+		nm_str_buf_erase (&request->in_buffer, 0, line_terminator - line + 1u, TRUE);
 	}
 
 out:
-	g_string_set_size (request->in_buffer, 0);
+	request->request_is_completed = TRUE;
+	nm_clear_g_source_inst (&request->child_stdout_watch_source);
+	if (   auth_result
+	    && request->child_stdin_watch_source) {
+		/* we need to wait for the buffer to send the response. */
+	} else
+		auth_request_complete (request, auth_result);
 
-	if (complete_auth) {
-		complete_authentication (request, auth_result);
-	}
 	return G_SOURCE_CONTINUE;
 }
 
@@ -553,7 +558,7 @@ begin_authentication (AuthRequest *request)
 	};
 
 	if (!request->username) {
-		complete_authentication (request, FALSE);
+		auth_request_complete (request, FALSE);
 		return;
 	}
 
@@ -576,7 +581,7 @@ begin_authentication (AuthRequest *request)
 		               0,
 		               "The PolicyKit setuid helper 'polkit-agent-helper-1' has not been found");
 
-		complete_authentication (request, FALSE);
+		auth_request_complete (request, FALSE);
 		return;
 	}
 
@@ -623,19 +628,21 @@ create_request (NMPolkitListener *listener,
                 char *username_take,
                 const char *cookie)
 {
-	AuthRequest *request = g_slice_new0(AuthRequest);
+	AuthRequest *request;
 
-	request->listener = listener;
-	request->dbus_invocation = invocation;
-	request->action_id = g_strdup (action_id);
-	request->message = g_strdup (message);
-	request->username = g_steal_pointer (&username_take);
-	request->cookie = g_strdup (cookie);
-	request->in_buffer = g_string_new ("");
+	request = g_slice_new (AuthRequest);
+	*request = (AuthRequest) {
+		.listener             = listener,
+		.dbus_invocation      = invocation,
+		.action_id            = g_strdup (action_id),
+		.message              = g_strdup (message),
+		.username             = g_steal_pointer (&username_take),
+		.cookie               = g_strdup (cookie),
+		.request_any_response = FALSE,
+		.request_is_completed = FALSE,
+	};
 
-	/* preallocate a large enough buffer so that
-	 * secrets don't get reallocated, thus leaked */
-	request->out_buffer = g_string_sized_new (1024);
+	nm_str_buf_init (&request->in_buffer,  NM_UTILS_GET_NEXT_REALLOC_SIZE_1000, FALSE);
 
 	c_list_link_tail (&listener->request_lst_head, &request->request_lst);
 	return request;
@@ -696,7 +703,7 @@ dbus_method_call_cb (GDBusConnection *connection,
 		}
 
 		/* Complete a cancelled request with success. */
-		complete_authentication (request, TRUE);
+		auth_request_complete (request, TRUE);
 		return;
 	}
 
@@ -895,9 +902,8 @@ dispose (GObject *object)
 
 	nm_clear_g_cancellable (&self->cancellable);
 
-	while ((request = c_list_first_entry (&self->request_lst_head, AuthRequest, request_lst))) {
-		remove_request (request);
-	}
+	while ((request = c_list_first_entry (&self->request_lst_head, AuthRequest, request_lst)))
+		auth_request_complete (request, FALSE);
 
 	if (self->dbus_connection) {
 		nm_clear_g_dbus_connection_signal (self->dbus_connection,
diff --git a/shared/nm-glib-aux/nm-io-utils.c b/shared/nm-glib-aux/nm-io-utils.c
index 9620c688df..776c63e111 100644
--- a/shared/nm-glib-aux/nm-io-utils.c
+++ b/shared/nm-glib-aux/nm-io-utils.c
@@ -11,6 +11,7 @@
 #include <sys/stat.h>
 #include <fcntl.h>
 
+#include "nm-str-buf.h"
 #include "nm-shared-utils.h"
 #include "nm-secret-utils.h"
 #include "nm-errno.h"
@@ -418,27 +419,40 @@ nm_utils_file_stat (const char *filename, struct stat *out_st)
  * @fd: the fd to read from.
  * @out_string: (out): output string where read bytes will be stored.
  *
- * Returns:  <0 on failure, which is -(errno)
- *           0 on EOF or if the call would block (if the fd is nonblocking),
- *           >0 on success, which is the number of bytes read  */
-ssize_t
-nm_utils_fd_read (int fd, GString *out_string)
+ * Returns: <0 on failure, which is -(errno).
+ *          0 on EOF.
+ *          >0 on success, which is the number of bytes read.  */
+gssize
+nm_utils_fd_read (int fd, NMStrBuf *out_string)
 {
-	size_t start_len;
-	ssize_t n_read;
+	gsize buf_available;
+	gssize n_read;
+	int errsv;
 
 	g_return_val_if_fail (fd >= 0, -1);
 	g_return_val_if_fail (out_string, -1);
 
-	start_len = out_string->len;
-	g_string_set_size (out_string, start_len + 1024);
-
-	n_read = read (fd, &out_string->str[start_len], 1024);
-	if (n_read < 0)
-		return -NM_ERRNO_NATIVE (errno);
+	/* If the buffer size is 0, we allocate NM_UTILS_GET_NEXT_REALLOC_SIZE_1000 (1000 bytes)
+	 * the first time. Afterwards, the buffer grows exponentially.
+	 *
+	 * Note that with @buf_available, we always would read as much buffer as we actually
+	 * have reserved. */
+	nm_str_buf_maybe_expand (out_string, NM_UTILS_GET_NEXT_REALLOC_SIZE_1000, FALSE);
+
+	buf_available = out_string->allocated - out_string->len;
+
+	n_read = read (fd,
+	               &((nm_str_buf_get_str_unsafe (out_string))[out_string->len]),
+	               buf_available);
+	if (n_read < 0) {
+		errsv = errno;
+		return -NM_ERRNO_NATIVE (errsv);
+	}
 
-	if (n_read > 0)
-		g_string_set_size (out_string, start_len + (gsize) n_read);
+	if (n_read > 0) {
+		nm_assert ((gsize) n_read <= buf_available);
+		nm_str_buf_set_size (out_string, out_string->len + (gsize) n_read, TRUE, FALSE);
+	}
 
 	return n_read;
 }
diff --git a/shared/nm-glib-aux/nm-io-utils.h b/shared/nm-glib-aux/nm-io-utils.h
index dd2f499d6f..31326fc79a 100644
--- a/shared/nm-glib-aux/nm-io-utils.h
+++ b/shared/nm-glib-aux/nm-io-utils.h
@@ -47,7 +47,9 @@ gboolean nm_utils_file_set_contents (const char *filename,
                                      int *out_errsv,
                                      GError **error);
 
-ssize_t nm_utils_fd_read (int fd, GString *out_string);
+struct _NMStrBuf;
+
+gssize nm_utils_fd_read (int fd, struct _NMStrBuf *out_string);
 
 struct stat;