Don't write X authority with g_file_replace - it can leave intermediate files around if it is interrupted.

9 files changed, 105 insertions, 161 deletions

diff --git a/src/seat-xlocal.c b/src/seat-xlocal.c
index a1fb2e01..026ce392 100644
--- a/src/seat-xlocal.c
+++ b/src/seat-xlocal.c
@@ -134,14 +134,13 @@ seat_xlocal_set_active_display (Seat *seat, Display *display)
 static void
 seat_xlocal_run_script (Seat *seat, Display *display, Process *script)
 {
-    gchar *path;
+    const gchar *path;
     XServerLocal *xserver;
 
     xserver = XSERVER_LOCAL (display_get_display_server (display));
     path = xserver_local_get_authority_file_path (xserver);
     process_set_env (script, "DISPLAY", xserver_get_address (XSERVER (xserver)));
     process_set_env (script, "XAUTHORITY", path);
-    g_free (path);
 
     SEAT_CLASS (seat_xlocal_parent_class)->run_script (seat, display, script);
 }
diff --git a/src/seat-xvnc.c b/src/seat-xvnc.c
index eb150c31..a53b89db 100644
--- a/src/seat-xvnc.c
+++ b/src/seat-xvnc.c
@@ -85,7 +85,7 @@ seat_xvnc_run_script (Seat *seat, Display *display, Process *script)
     XServerXVNC *xserver;
     GInetSocketAddress *address;
     gchar *hostname;
-    gchar *path;
+    const gchar *path;
 
     xserver = XSERVER_XVNC (display_get_display_server (display));
 
@@ -98,7 +98,6 @@ seat_xvnc_run_script (Seat *seat, Display *display, Process *script)
     process_set_env (script, "XAUTHORITY", path);
 
     g_free (hostname);
-    g_free (path);
 
     SEAT_CLASS (seat_xvnc_parent_class)->run_script (seat, display, script);
 }
diff --git a/src/session-child.c b/src/session-child.c
index c29924cd..fdf9a22b 100644
--- a/src/session-child.c
+++ b/src/session-child.c
@@ -435,21 +435,17 @@ session_child_run (int argc, char **argv)
     /* Write X authority */
     if (xauthority)
     {
-        GFile *file;
         gboolean drop_privileges, result;
         gchar *value;
         GError *error = NULL;
 
-        file = g_file_new_for_path (xauth_filename);
-
         drop_privileges = geteuid () == 0;
         if (drop_privileges)
             privileges_drop (user);
-        result = xauth_write (xauthority, XAUTH_WRITE_MODE_REPLACE, file, &error);
+        result = xauth_write (xauthority, XAUTH_WRITE_MODE_REPLACE, xauth_filename, &error);
         if (drop_privileges)
             privileges_reclaim ();
 
-        g_object_unref (file);
         if (error)
             g_printerr ("Error writing X authority: %s\n", error->message);
         g_clear_error (&error);
@@ -589,20 +585,16 @@ session_child_run (int argc, char **argv)
     /* Remove X authority */
     if (xauthority)
     {
-        GFile *file;
         gboolean drop_privileges, result;
         GError *error = NULL;
 
-        file = g_file_new_for_path (xauth_filename);
-
         drop_privileges = geteuid () == 0;
         if (drop_privileges)
             privileges_drop (user);
-        result = xauth_write (xauthority, XAUTH_WRITE_MODE_REMOVE, file, &error);
+        result = xauth_write (xauthority, XAUTH_WRITE_MODE_REMOVE, xauth_filename, &error);
         if (drop_privileges)
             privileges_reclaim ();
 
-        g_object_unref (file);
         if (error)
             g_printerr ("Error removing X authority: %s\n", error->message);
         g_clear_error (&error);
diff --git a/src/xauthority.c b/src/xauthority.c
index 1911b904..fd031b0d 100644
--- a/src/xauthority.c
+++ b/src/xauthority.c
@@ -10,6 +10,7 @@
  */
 
 #include <string.h>
+#include <stdio.h>
 #include <errno.h>
 #include <unistd.h>
 #include <sys/stat.h>
@@ -169,119 +170,111 @@ xauth_get_authorization_data_length (XAuthority *auth)
 }
 
 static gboolean
-read_uint16 (GInputStream *stream, guint16 *value, gboolean *eof, GError **error)
+read_uint16 (gchar *data, gsize data_length, gsize *offset, guint16 *value)
 {
-    guint8 data[2] = {0, 0};
-    gsize n_read;
-
-    if (g_input_stream_read_all (stream, data, 2, &n_read, NULL, error) < 0)
+    if (data_length - *offset < 2)
         return FALSE;
-  
-    if (n_read == 0 && eof)
-       *eof = TRUE;
 
-    *value = data[0] << 8 | data[1];
-  
+    *value = data[*offset] << 8 | data[*offset + 1];
+    *offset += 2;
+
     return TRUE;
 }
 
 static gboolean
-read_data (GInputStream *stream, guint16 length, guint8 **value, GError **error)
+read_data (gchar *data, gsize data_length, gsize *offset, guint16 length, guint8 **value)
 {
+    int i;
+
     g_free (*value);
-    *value = g_malloc0 (length + 1);
-    if (g_input_stream_read_all (stream, *value, length, NULL, NULL, error) < 0)
-    {
-        g_free (*value);
-        *value = NULL;
+    *value = NULL;
+
+    if (data_length - *offset < length)
         return FALSE;
-    }
+  
+    *value = g_malloc0 (length + 1);
+    for (i = 0; i < length; i++)
+        (*value)[i] = data[*offset + i];
+    *offset += length;
     (*value)[length] = 0;
 
     return TRUE;
 }
 
 static gboolean
-read_string (GInputStream *stream, gchar **value, GError **error)
+read_string (gchar *data, gsize data_length, gsize *offset, gchar **value)
 {
     guint16 length;
-    if (!read_uint16 (stream, &length, NULL, error))
+    if (!read_uint16 (data, data_length, offset, &length))
         return FALSE;
-    return read_data (stream, length, (guint8 **) value, error);
+    return read_data (data, data_length, offset, length, (guint8 **) value);
 }
 
-static gboolean
-write_uint16 (GOutputStream *stream, guint16 value, GError **error)
+static void
+write_uint16 (FILE *file, guint16 value)
 {
-    guint8 data[2];
-
-    data[0] = value >> 8;
-    data[1] = value & 0xFF;
-    return g_output_stream_write (stream, data, 2, NULL, error) >= 0;
+    guint8 v[2];
+    v[0] = value >> 8;
+    v[1] = value & 0xFF;
+    fwrite (v, 2, 1, file);
 }
 
-static gboolean
-write_data (GOutputStream *stream, const guint8 *data, gsize data_length, GError **error)
+static void
+write_data (FILE *file, const guint8 *value, gsize value_length)
 {
-    return g_output_stream_write (stream, data, data_length, NULL, error) >= 0;
+    fwrite (value, value_length, 1, file);
 }
 
-static gboolean
-write_string (GOutputStream *stream, const gchar *value, GError **error)
+static void
+write_string (FILE *file, const gchar *value)
 {
-    if (!write_uint16 (stream, strlen (value), error) ||
-        !write_data (stream, (guint8 *) value, strlen (value), error))
-        return FALSE;
-  
-    return TRUE;
+    write_uint16 (file, strlen (value));
+    write_data (file, (guint8 *) value, strlen (value));
 }
 
 gboolean
-xauth_write (XAuthority *auth, XAuthWriteMode mode, GFile *file, GError **error)
+xauth_write (XAuthority *auth, XAuthWriteMode mode, const gchar *filename, GError **error)
 {
+    gchar *input;
+    gsize input_length = 0, input_offset = 0;
     GList *link, *records = NULL;
-    GFileInputStream *input_stream = NULL;
-    GFileOutputStream *output_stream;
     XAuthority *a;
     gboolean result;
     gboolean matched = FALSE;
+    FILE *output;
 
     g_return_val_if_fail (auth != NULL, FALSE);
-    g_return_val_if_fail (file != NULL, FALSE);
+    g_return_val_if_fail (filename != NULL, FALSE);
 
     /* Read out existing records */
     if (mode != XAUTH_WRITE_MODE_SET)
     {
         GError *read_error = NULL;
 
-        input_stream = g_file_read (file, NULL, &read_error);
-        if (read_error && !g_error_matches (read_error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND))
+        g_file_get_contents (filename, &input, &input_length, &read_error);
+        if (read_error && !g_error_matches (read_error, G_FILE_ERROR, G_FILE_ERROR_NOENT))
             g_warning ("Error reading existing Xauthority: %s", read_error->message);
         g_clear_error (&read_error);
     }
-    while (input_stream)
+    while (input_offset != input_length)
     {
-        gboolean eof = FALSE, address_matches = FALSE;
+        gboolean address_matches = FALSE;
         guint16 address_length = 0;
         guint16 authorization_data_length = 0;
-        GError *read_error = NULL;
 
         a = g_object_new (XAUTHORITY_TYPE, NULL);
 
-        result = read_uint16 (G_INPUT_STREAM (input_stream), &a->priv->family, &eof, &read_error) &&
-                 read_uint16 (G_INPUT_STREAM (input_stream), &address_length, NULL, &read_error) &&
-                 read_data (G_INPUT_STREAM (input_stream), address_length, &a->priv->address, &read_error) &&
-                 read_string (G_INPUT_STREAM (input_stream), &a->priv->number, &read_error) &&
-                 read_string (G_INPUT_STREAM (input_stream), &a->priv->authorization_name, &read_error) &&
-                 read_uint16 (G_INPUT_STREAM (input_stream), &authorization_data_length, NULL, &read_error) &&
-                 read_data (G_INPUT_STREAM (input_stream), authorization_data_length, &a->priv->authorization_data, &read_error);
+        result = read_uint16 (input, input_length, &input_offset, &a->priv->family) &&
+                 read_uint16 (input, input_length, &input_offset, &address_length) &&
+                 read_data (input, input_length, &input_offset, address_length, &a->priv->address) &&
+                 read_string (input, input_length, &input_offset, &a->priv->number) &&
+                 read_string (input, input_length, &input_offset, &a->priv->authorization_name) &&
+                 read_uint16 (input, input_length, &input_offset, &authorization_data_length) &&
+                 read_data (input, input_length, &input_offset, authorization_data_length, &a->priv->authorization_data);
         a->priv->address_length = address_length;
         a->priv->authorization_data_length = authorization_data_length;
-        if (read_error)
-            g_warning ("Error reading X authority %s: %s", g_file_get_path (file), read_error->message);
-        g_clear_error (&read_error);
 
-        if (eof || !result)
+        if (!result)
         {
             g_object_unref (a);
             break;
@@ -312,50 +305,45 @@ xauth_write (XAuthority *auth, XAuthWriteMode mode, GFile *file, GError **error)
 
         records = g_list_append (records, a);
     }
-    if (input_stream)
-    {
-        GError *close_error = NULL;
-        g_input_stream_close (G_INPUT_STREAM (input_stream), NULL, &close_error);
-        if (close_error)
-            g_warning ("Error closing Xauthority: %s", close_error->message);
-        g_clear_error (&close_error);
-        g_object_unref (input_stream);
-    }
+    g_free (input);
 
     /* If didn't exist, then add a new one */
     if (!matched)
         records = g_list_append (records, g_object_ref (auth));
 
-    output_stream = g_file_replace (file, NULL, FALSE, G_FILE_CREATE_PRIVATE, NULL, error);
-    if (!output_stream)
-        return FALSE;
-
-    /* Workaround because g_file_replace () generates a file does not exist error even though it can replace it */
-    g_clear_error (error);
-
     /* Write records back */
-    result = TRUE;
+    errno = 0;
+    output = fopen (filename, "w");
+    if (output == NULL)
+    {
+        g_set_error (error,
+                     G_FILE_ERROR,
+                     g_file_error_from_errno (errno),
+                     "Failed to write X authority %s: %s",
+                     filename,
+                     g_strerror (errno));
+        return FALSE;
+    }
+  
     for (link = records; link && result; link = link->next)
     {
         XAuthority *a = link->data;
 
-        result = write_uint16 (G_OUTPUT_STREAM (output_stream), a->priv->family, error) &&
-                 write_uint16 (G_OUTPUT_STREAM (output_stream), a->priv->address_length, error) &&
-                 write_data (G_OUTPUT_STREAM (output_stream), a->priv->address, a->priv->address_length, error) &&
-                 write_string (G_OUTPUT_STREAM (output_stream), a->priv->number, error) &&
-                 write_string (G_OUTPUT_STREAM (output_stream), a->priv->authorization_name, error) &&
-                 write_uint16 (G_OUTPUT_STREAM (output_stream), a->priv->authorization_data_length, error) &&
-                 write_data (G_OUTPUT_STREAM (output_stream), a->priv->authorization_data, a->priv->authorization_data_length, error);
+        write_uint16 (output, a->priv->family);
+        write_uint16 (output, a->priv->address_length);
+        write_data (output, a->priv->address, a->priv->address_length);
+        write_string (output, a->priv->number);
+        write_string (output, a->priv->authorization_name);
+        write_uint16 (output, a->priv->authorization_data_length);
+        write_data (output, a->priv->authorization_data, a->priv->authorization_data_length);
 
         g_object_unref (a);
     }
     g_list_free (records);
 
-    if (result)
-        result = g_output_stream_close (G_OUTPUT_STREAM (output_stream), NULL, error);
-    g_object_unref (output_stream);
+    fclose (output);
 
-    return result;
+    return TRUE;
 }    
 
 static void
diff --git a/src/xauthority.h b/src/xauthority.h
index 2a190c09..32f1133d 100644
--- a/src/xauthority.h
+++ b/src/xauthority.h
@@ -13,7 +13,6 @@
 #define _XAUTHORITY_H_
 
 #include <glib-object.h>
-#include <gio/gio.h>
 
 G_BEGIN_DECLS
 
@@ -83,7 +82,7 @@ guint8 *xauth_copy_authorization_data (XAuthority *auth);
 
 gsize xauth_get_authorization_data_length (XAuthority *auth);
 
-gboolean xauth_write (XAuthority *auth, XAuthWriteMode mode, GFile *file, GError **error);
+gboolean xauth_write (XAuthority *auth, XAuthWriteMode mode, const gchar *filename, GError **error);
 
 G_END_DECLS
 
diff --git a/src/xserver-local.c b/src/xserver-local.c
index e8749b69..210d231e 100644
--- a/src/xserver-local.c
+++ b/src/xserver-local.c
@@ -15,6 +15,7 @@
 #include <sys/stat.h>
 #include <errno.h>
 #include <glib/gstdio.h>
+#include <unistd.h>
 
 #include "xserver-local.h"
 #include "configuration.h"
@@ -43,7 +44,7 @@ struct XServerLocalPrivate
     gboolean allow_tcp;
 
     /* Authority file */
-    GFile *authority_file;
+    gchar *authority_file;
 
     /* XDMCP server to connect to */
     gchar *xdmcp_server;
@@ -228,13 +229,11 @@ xserver_local_get_vt (XServerLocal *server)
     return server->priv->vt;
 }
 
-gchar *
+const gchar *
 xserver_local_get_authority_file_path (XServerLocal *server)
 {
-    g_return_val_if_fail (server != NULL, 0);
-    if (server->priv->authority_file)
-        return g_file_get_path (server->priv->authority_file);
-    return NULL;
+    g_return_val_if_fail (server != NULL, NULL);
+    return server->priv->authority_file;
 }
 
 static gchar *
@@ -322,19 +321,11 @@ stopped_cb (Process *process, XServerLocal *server)
   
     if (xserver_get_authority (XSERVER (server)) && server->priv->authority_file)
     {
-        GError *error = NULL;
-        gchar *path;
-
-        path = g_file_get_path (server->priv->authority_file);      
-        g_debug ("Removing X server authority %s", path);
-        g_free (path);
+        g_debug ("Removing X server authority %s", server->priv->authority_file);
 
-        g_file_delete (server->priv->authority_file, NULL, &error);
-        if (error)
-            g_debug ("Error removing authority: %s", error->message);
-        g_clear_error (&error);
+        g_unlink (server->priv->authority_file);
 
-        g_object_unref (server->priv->authority_file);
+        g_free (server->priv->authority_file);
         server->priv->authority_file = NULL;
     }
 
@@ -358,7 +349,6 @@ static void
 write_authority_file (XServerLocal *server)
 {
     XAuthority *authority;
-    gchar *path;
     GError *error = NULL;
 
     authority = xserver_get_authority (XSERVER (server));
@@ -375,15 +365,11 @@ write_authority_file (XServerLocal *server)
         g_free (run_dir);
         g_mkdir_with_parents (dir, S_IRWXU);
 
-        path = g_build_filename (dir, xserver_get_address (XSERVER (server)), NULL);
+        server->priv->authority_file = g_build_filename (dir, xserver_get_address (XSERVER (server)), NULL);
         g_free (dir);
-        server->priv->authority_file = g_file_new_for_path (path);
-        g_free (path);
     }
 
-    path = g_file_get_path (server->priv->authority_file);
-    g_debug ("Writing X server authority to %s", path);
-    g_free (path);
+    g_debug ("Writing X server authority to %s", server->priv->authority_file);
 
     xauth_write (authority, XAUTH_WRITE_MODE_REPLACE, server->priv->authority_file, &error);
     if (error)
@@ -445,11 +431,7 @@ xserver_local_start (DisplayServer *display_server)
     g_free (number);
     write_authority_file (server);
     if (server->priv->authority_file)
-    {
-        gchar *path = g_file_get_path (server->priv->authority_file);
-        g_string_append_printf (command, " -auth %s", path);
-        g_free (path);
-    }
+        g_string_append_printf (command, " -auth %s", server->priv->authority_file);
 
     /* Connect to a remote server using XDMCP */
     if (server->priv->xdmcp_server != NULL)
@@ -541,8 +523,7 @@ xserver_local_finalize (GObject *object)
     g_free (self->priv->layout);
     g_free (self->priv->xdmcp_server);
     g_free (self->priv->xdmcp_key);
-    if (self->priv->authority_file)
-        g_object_unref (self->priv->authority_file);
+    g_free (self->priv->authority_file);
     if (self->priv->vt >= 0)
         vt_unref (self->priv->vt);
 
diff --git a/src/xserver-local.h b/src/xserver-local.h
index 0d07e4b2..b2925ba6 100644
--- a/src/xserver-local.h
+++ b/src/xserver-local.h
@@ -63,7 +63,7 @@ void xserver_local_set_xdmcp_key (XServerLocal *server, const gchar *key);
 
 gint xserver_local_get_vt (XServerLocal *server);
 
-gchar *xserver_local_get_authority_file_path (XServerLocal *server);
+const gchar *xserver_local_get_authority_file_path (XServerLocal *server);
 
 G_END_DECLS
 
diff --git a/src/xserver-xvnc.c b/src/xserver-xvnc.c
index ef6b7f3a..766bc92b 100644
--- a/src/xserver-xvnc.c
+++ b/src/xserver-xvnc.c
@@ -15,6 +15,7 @@
 #include <sys/stat.h>
 #include <errno.h>
 #include <glib/gstdio.h>
+#include <unistd.h>
 
 #include "xserver-xvnc.h"
 #include "configuration.h"
@@ -30,7 +31,7 @@ struct XServerXVNCPrivate
     gchar *log_file;  
 
     /* Authority file */
-    GFile *authority_file;
+    gchar *authority_file;
 
     /* File descriptor to use for standard input */
     gint socket_fd;
@@ -88,13 +89,11 @@ xserver_xvnc_set_depth (XServerXVNC *server, gint depth)
     server->priv->depth = depth;
 }
 
-gchar *
+const gchar *
 xserver_xvnc_get_authority_file_path (XServerXVNC *server)
 {
-    g_return_val_if_fail (server != NULL, 0);
-    if (server->priv->authority_file)
-        return g_file_get_path (server->priv->authority_file);
-    return NULL;
+    g_return_val_if_fail (server != NULL, NULL);
+    return server->priv->authority_file;
 }
 
 static gchar *
@@ -162,9 +161,6 @@ got_signal_cb (Process *process, int signum, XServerXVNC *server)
 static void
 stopped_cb (Process *process, XServerXVNC *server)
 {
-    GError *error = NULL;
-    gchar *path;
-  
     g_debug ("Xvnc server stopped");
 
     g_object_unref (server->priv->xserver_process);
@@ -172,15 +168,10 @@ stopped_cb (Process *process, XServerXVNC *server)
 
     xserver_local_release_display_number (xserver_get_display_number (XSERVER (server)));
 
-    path = g_file_get_path (server->priv->authority_file);
-    g_debug ("Removing X server authority %s", path);
-    g_free (path);
+    g_debug ("Removing X server authority %s", server->priv->authority_file);
 
-    g_file_delete (server->priv->authority_file, NULL, &error);
-    if (error)
-        g_debug ("Error removing authority: %s", error->message);
-    g_clear_error (&error);
-    g_object_unref (server->priv->authority_file);
+    g_unlink (server->priv->authority_file);
+    g_free (server->priv->authority_file);
     server->priv->authority_file = NULL;
 
     DISPLAY_SERVER_CLASS (xserver_xvnc_parent_class)->stop (DISPLAY_SERVER (server));
@@ -192,7 +183,7 @@ xserver_xvnc_start (DisplayServer *display_server)
     XServerXVNC *server = XSERVER_XVNC (display_server);
     XAuthority *authority;
     gboolean result;
-    gchar *filename, *run_dir, *dir, *path, *absolute_command;
+    gchar *filename, *run_dir, *dir, *absolute_command;
     GString *command;
     gchar hostname[1024], *number;
     GError *error = NULL;
@@ -234,13 +225,10 @@ xserver_xvnc_start (DisplayServer *display_server)
     g_free (run_dir);
     g_mkdir_with_parents (dir, S_IRWXU);
 
-    path = g_build_filename (dir, xserver_get_address (XSERVER (server)), NULL);
+    server->priv->authority_file = g_build_filename (dir, xserver_get_address (XSERVER (server)), NULL);
     g_free (dir);
-    server->priv->authority_file = g_file_new_for_path (path);
-    g_free (path);
 
-    path = g_file_get_path (server->priv->authority_file);
-    g_debug ("Writing X server authority to %s", path);
+    g_debug ("Writing X server authority to %s", server->priv->authority_file);
 
     xauth_write (authority, XAUTH_WRITE_MODE_REPLACE, server->priv->authority_file, &error);
     if (error)
@@ -251,8 +239,7 @@ xserver_xvnc_start (DisplayServer *display_server)
     g_free (absolute_command);
   
     g_string_append_printf (command, " :%d", xserver_get_display_number (XSERVER (server)));
-    g_string_append_printf (command, " -auth %s", path);
-    g_free (path);
+    g_string_append_printf (command, " -auth %s", server->priv->authority_file);
     g_string_append (command, " -inetd -nolisten tcp");
     if (server->priv->width > 0 && server->priv->height > 0)
         g_string_append_printf (command, " -geometry %dx%d", server->priv->width, server->priv->height);
@@ -312,8 +299,7 @@ xserver_xvnc_finalize (GObject *object)
 
     if (self->priv->xserver_process)
         g_object_unref (self->priv->xserver_process);
-    if (self->priv->authority_file)
-        g_object_unref (self->priv->authority_file);
+    g_free (self->priv->authority_file);
     g_free (self->priv->log_file);
 
     G_OBJECT_CLASS (xserver_xvnc_parent_class)->finalize (object);
diff --git a/src/xserver-xvnc.h b/src/xserver-xvnc.h
index b7893630..48454f49 100644
--- a/src/xserver-xvnc.h
+++ b/src/xserver-xvnc.h
@@ -49,7 +49,7 @@ void xserver_xvnc_set_geometry (XServerXVNC *server, gint width, gint height);
 
 void xserver_xvnc_set_depth (XServerXVNC *server, gint depth);
 
-gchar *xserver_xvnc_get_authority_file_path (XServerXVNC *server);
+const gchar *xserver_xvnc_get_authority_file_path (XServerXVNC *server);
 
 G_END_DECLS