[tracker] libtracker-extract: Crash in miner-fs due to size_t vs gssize

[Martyn James Russell <mr src gnome org>]

commit e635ba4afef89e16922caa84d8aac4dccc8bc873
Author: Martyn Russell <martyn lanedo com>
Date:   Wed Feb 15 09:45:48 2012 +0000

    libtracker-extract: Crash in miner-fs due to size_t vs gssize
    
    Fixed by making use of GInputStream/GDataInputStream on the receiving end
    (which we were doing on the sending end) to avoid use of strlen().
    
    Crash was occurring on OpenBSD.
    
    Fixes GB#669546.

 src/libtracker-extract/tracker-extract-client.c |   93 +++++++++++++++++-----
 src/tracker-extract/tracker-controller.c        |   93 ++++++++++-------------
 2 files changed, 112 insertions(+), 74 deletions(-)
---
diff --git a/src/libtracker-extract/tracker-extract-client.c b/src/libtracker-extract/tracker-extract-client.c
index 42d82bb..4e2746b 100644
--- a/src/libtracker-extract/tracker-extract-client.c
+++ b/src/libtracker-extract/tracker-extract-client.c
@@ -18,14 +18,16 @@
  */
 
 #include "config.h"
-#include "tracker-extract-client.h"
 
 #include <string.h>
 #include <unistd.h>
 #include <errno.h>
+
 #include <gio/gunixfdlist.h>
 #include <gio/gunixinputstream.h>
 
+#include "tracker-extract-client.h"
+
 /* Size of buffers used when sending data over a pipe, using DBus FD passing */
 #define DBUS_PIPE_BUFFER_SIZE      65536
 
@@ -270,6 +272,49 @@ dbus_send_and_splice_async (GDBusConnection       *connection,
 	                              data);
 }
 
+static inline const gchar *
+get_metadata_fast_read (GDataInputStream *data_input_stream,
+                        gsize            *remaining,
+                        GError           *error)
+{
+	const gchar *output;
+	gsize len_read;
+
+	if (error) {
+		return NULL;
+	}
+
+	g_return_val_if_fail (*remaining > 0, NULL);
+
+	/* Read data */
+	output = g_data_input_stream_read_upto (data_input_stream, "\0", 1, &len_read, NULL, &error);
+
+	if (error) {
+		return NULL;
+	}
+
+	*remaining -= len_read;
+
+	g_return_val_if_fail (*remaining > 0, NULL);
+
+	/* Read NUL terminating byte.
+	 *
+	 * The g_data_input_stream_read_upto() function doesn't
+	 * consume the bytes we read up to according to the
+	 * documentation unlike the _until() variant which is now
+	 * deprecated anyway.
+	 */
+	g_data_input_stream_read_byte (data_input_stream, NULL, &error);
+
+	if (error) {
+		return NULL;
+	}
+
+	*remaining -= 1;
+
+	return output;
+}
+
 static void
 get_metadata_fast_cb (void     *buffer,
                       gssize    buffer_size,
@@ -283,31 +328,37 @@ get_metadata_fast_cb (void     *buffer,
 	if (G_UNLIKELY (error)) {
 		g_simple_async_result_set_from_error (data->res, error);
 	} else {
-		const gchar *preupdate, *postupdate, *sparql, *where, *end;
+		GInputStream *input_stream;
+		GDataInputStream *data_input_stream;
+		const gchar *preupdate, *postupdate, *sparql, *where;
 		TrackerSparqlBuilder *builder;
-		gsize len;
-
+		gssize remaining;
+
+		/* So the structure is like this:
+		 *
+		 *   [buffer,'\0'][buffer,'\0'][...]
+		 *
+		 * We avoid strlen() using
+		 * g_data_input_stream_read_upto() and the
+		 * NUL-terminating byte given strlen() has a size_t
+		 * limitation and costs us time evaluating string
+		 * lengths.
+		 */
 		preupdate = postupdate = sparql = where = NULL;
-		end = (gchar *) buffer + buffer_size;
-
-		if (buffer) {
-			preupdate = buffer;
-			len = strlen (preupdate);
+		remaining = buffer_size;
 
-			if (preupdate + len < end) {
-				postupdate = preupdate + len + 1;
-				len = strlen (postupdate);
+		input_stream = g_memory_input_stream_new_from_data (buffer, buffer_size, NULL);
+		data_input_stream = g_data_input_stream_new (input_stream);
+		g_data_input_stream_set_byte_order (G_DATA_INPUT_STREAM (data_input_stream),
+		                                    G_DATA_STREAM_BYTE_ORDER_HOST_ENDIAN);
 
-				if (postupdate + len < end) {
-					sparql = postupdate + len + 1;
-					len = strlen (sparql);
+		preupdate  = get_metadata_fast_read (data_input_stream, &remaining, error);
+		postupdate = get_metadata_fast_read (data_input_stream, &remaining, error);
+		sparql     = get_metadata_fast_read (data_input_stream, &remaining, error);
+		where      = get_metadata_fast_read (data_input_stream, &remaining, error);
 
-					if (sparql + len < end) {
-						where = sparql + len + 1;
-					}
-				}
-			}
-		}
+		g_object_unref (data_input_stream);
+		g_object_unref (input_stream);
 
 		if (where) {
 			tracker_extract_info_set_where_clause (data->info, where);
diff --git a/src/tracker-extract/tracker-controller.c b/src/tracker-extract/tracker-controller.c
index a2b0a0f..4bc3b2a 100644
--- a/src/tracker-extract/tracker-controller.c
+++ b/src/tracker-extract/tracker-controller.c
@@ -559,6 +559,32 @@ handle_method_call_cancel_tasks (TrackerController     *controller,
 	g_dbus_method_invocation_return_value (invocation, NULL);
 }
 
+static inline void
+get_metadata_fast_write (GDataOutputStream *data_output_stream,
+                         const gchar       *string,
+                         GError            *error)
+{
+	if (error) {
+		return;
+	}
+
+	/* Append data */
+	g_data_output_stream_put_string (data_output_stream,
+	                                 string ? string : "",
+	                                 NULL,
+	                                 &error);
+
+	if (error) {
+		return;
+	}
+
+	/* Append a '\0' */
+	g_data_output_stream_put_byte (data_output_stream,
+	                               0,
+	                               NULL,
+	                               &error);
+}
+
 static void
 get_metadata_fast_cb (GObject      *object,
                       GAsyncResult *res,
@@ -604,60 +630,21 @@ get_metadata_fast_cb (GObject      *object,
 
 		where = tracker_extract_info_get_where_clause (info);
 
+		/* So the structure is like this:
+		 *
+		 *   [buffer,'\0'][buffer,'\0'][...]
+		 *
+		 * We avoid strlen() using
+		 * g_data_input_stream_read_upto() and the
+		 * NUL-terminating byte given strlen() has a size_t
+		 * limitation and costs us time evaluating string
+		 * lengths.
+		 */
 		if (statements && *statements) {
-			g_data_output_stream_put_string (data_output_stream,
-			                                 preupdate ? preupdate : "",
-			                                 NULL,
-			                                 &error);
-
-			if (!error) {
-				g_data_output_stream_put_byte (data_output_stream,
-				                               0,
-				                               NULL,
-				                               &error);
-			}
-
-			if (!error) {
-				g_data_output_stream_put_string (data_output_stream,
-				                                 postupdate ? postupdate : "",
-				                                 NULL,
-				                                 &error);
-			}
-
-			if (!error) {
-				g_data_output_stream_put_byte (data_output_stream,
-				                               0,
-				                               NULL,
-				                               &error);
-			}
-
-			if (!error) {
-				g_data_output_stream_put_string (data_output_stream,
-				                                 statements,
-				                                 NULL,
-				                                 &error);
-			}
-
-			if (!error) {
-				g_data_output_stream_put_byte (data_output_stream,
-				                               0,
-				                               NULL,
-				                               &error);
-			}
-
-			if (!error && where) {
-				g_data_output_stream_put_string (data_output_stream,
-				                                 where,
-				                                 NULL,
-				                                 &error);
-			}
-
-			if (!error) {
-				g_data_output_stream_put_byte (data_output_stream,
-				                               0,
-				                               NULL,
-				                               &error);
-			}
+			get_metadata_fast_write (data_output_stream, preupdate, error);
+			get_metadata_fast_write (data_output_stream, postupdate, error);
+			get_metadata_fast_write (data_output_stream, statements, error);
+			get_metadata_fast_write (data_output_stream, where, error);
 		}
 
 		g_object_unref (data_output_stream);