[glib] GConverterInputStream: fix an edge case



commit 69e12cd3d56ae43b188a278b807e517961627ada
Author: Dan Winship <danw gnome org>
Date:   Wed May 30 08:30:27 2012 -0400

    GConverterInputStream: fix an edge case
    
    Reading from a GConverterInputStream with both input_buffer and
    converted_buffer non-empty would return bogus data (the data from
    converted_buffer would essentially get skipped over, though the
    returned nread reflected what the count would be if it hadn't been).
    
    This was never noticed before because (a) it can't happen if all of
    your reads are at least as large as either the internal buffer size or
    the remaining length of the stream (which covers most real-world use),
    and (b) it can't happen if all of your reads are 1 byte (which covers
    most of tests/converter-test). (And (c) it only happens for some
    converters/input streams.) But this was happening occasionally in
    libsoup when content-sniffing a gzipped response, because the
    SoupContentSnifferStream would first read 512 bytes (to sniff), and
    then pass through larger reads after that.
    
    Fixed and added a test to converter-test.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=676478

 gio/gconverterinputstream.c  |    1 +
 gio/tests/converter-stream.c |  163 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 164 insertions(+), 0 deletions(-)
---
diff --git a/gio/gconverterinputstream.c b/gio/gconverterinputstream.c
index 1acf9a9..76ccc0a 100644
--- a/gio/gconverterinputstream.c
+++ b/gio/gconverterinputstream.c
@@ -415,6 +415,7 @@ read_internal (GInputStream *stream,
   buffer_read (&priv->converted_buffer, buffer, available);
 
   total_bytes_read = available;
+  buffer += available;
   count -= available;
 
   /* If there is no data to convert, and no pre-converted data,
diff --git a/gio/tests/converter-stream.c b/gio/tests/converter-stream.c
index ae1bdfb..d387f7b 100644
--- a/gio/tests/converter-stream.c
+++ b/gio/tests/converter-stream.c
@@ -564,6 +564,168 @@ test_compressor (void)
   g_object_unref (compressor);
 }
 
+#define LEFTOVER_SHORT_READ_SIZE 512
+
+#define G_TYPE_LEFTOVER_CONVERTER         (g_leftover_converter_get_type ())
+#define G_LEFTOVER_CONVERTER(o)           (G_TYPE_CHECK_INSTANCE_CAST ((o), G_TYPE_LEFTOVER_CONVERTER, GLeftoverConverter))
+#define G_LEFTOVER_CONVERTER_CLASS(k)     (G_TYPE_CHECK_CLASS_CAST((k), G_TYPE_LEFTOVER_CONVERTER, GLeftoverConverterClass))
+#define G_IS_LEFTOVER_CONVERTER(o)        (G_TYPE_CHECK_INSTANCE_TYPE ((o), G_TYPE_LEFTOVER_CONVERTER))
+#define G_IS_LEFTOVER_CONVERTER_CLASS(k)  (G_TYPE_CHECK_CLASS_TYPE ((k), G_TYPE_LEFTOVER_CONVERTER))
+#define G_LEFTOVER_CONVERTER_GET_CLASS(o) (G_TYPE_INSTANCE_GET_CLASS ((o), G_TYPE_LEFTOVER_CONVERTER, GLeftoverConverterClass))
+
+typedef struct _GLeftoverConverter       GLeftoverConverter;
+typedef struct _GLeftoverConverterClass  GLeftoverConverterClass;
+
+struct _GLeftoverConverterClass
+{
+  GObjectClass parent_class;
+};
+
+GType       g_leftover_converter_get_type (void) G_GNUC_CONST;
+GConverter *g_leftover_converter_new      (void);
+
+
+
+static void g_leftover_converter_iface_init          (GConverterIface *iface);
+
+struct _GLeftoverConverter
+{
+  GObject parent_instance;
+};
+
+G_DEFINE_TYPE_WITH_CODE (GLeftoverConverter, g_leftover_converter, G_TYPE_OBJECT,
+			 G_IMPLEMENT_INTERFACE (G_TYPE_CONVERTER,
+						g_leftover_converter_iface_init))
+
+static void
+g_leftover_converter_class_init (GLeftoverConverterClass *klass)
+{
+}
+
+static void
+g_leftover_converter_init (GLeftoverConverter *local)
+{
+}
+
+GConverter *
+g_leftover_converter_new (void)
+{
+  GConverter *conv;
+
+  conv = g_object_new (G_TYPE_LEFTOVER_CONVERTER, NULL);
+
+  return conv;
+}
+
+static void
+g_leftover_converter_reset (GConverter *converter)
+{
+}
+
+static GConverterResult
+g_leftover_converter_convert (GConverter *converter,
+			      const void *inbuf,
+			      gsize       inbuf_size,
+			      void       *outbuf,
+			      gsize       outbuf_size,
+			      GConverterFlags flags,
+			      gsize      *bytes_read,
+			      gsize      *bytes_written,
+			      GError    **error)
+{
+  if (outbuf_size == LEFTOVER_SHORT_READ_SIZE)
+    {
+      g_set_error_literal (error,
+			   G_IO_ERROR,
+			   G_IO_ERROR_PARTIAL_INPUT,
+			   "partial input");
+      return G_CONVERTER_ERROR;
+    }
+
+  if (inbuf_size < 100)
+    *bytes_read = *bytes_written = MIN (inbuf_size, outbuf_size);
+  else
+    *bytes_read = *bytes_written = MIN (inbuf_size - 10, outbuf_size);
+  memcpy (outbuf, inbuf, *bytes_written);
+
+  if (*bytes_read == inbuf_size && (flags & G_CONVERTER_INPUT_AT_END))
+    return G_CONVERTER_FINISHED;
+  return G_CONVERTER_CONVERTED;
+}
+
+static void
+g_leftover_converter_iface_init (GConverterIface *iface)
+{
+  iface->convert = g_leftover_converter_convert;
+  iface->reset = g_leftover_converter_reset;
+}
+
+#define LEFTOVER_BUFSIZE 8192
+#define INTERNAL_BUFSIZE 4096
+
+static void
+test_converter_leftover (void)
+{
+  gchar *orig, *converted;
+  gsize total_read;
+  gssize res;
+  goffset offset;
+  GInputStream *mem, *cstream;
+  GConverter *converter;
+  GError *error;
+  int i;
+
+  converter = g_leftover_converter_new ();
+
+  orig = g_malloc (LEFTOVER_BUFSIZE);
+  converted = g_malloc (LEFTOVER_BUFSIZE);
+  for (i = 0; i < LEFTOVER_BUFSIZE; i++)
+    orig[i] = i % 64 + 32;
+
+  mem = g_memory_input_stream_new_from_data (orig, LEFTOVER_BUFSIZE, NULL);
+  cstream = g_converter_input_stream_new (mem, G_CONVERTER (converter));
+  g_object_unref (mem);
+
+  total_read = 0;
+
+  error = NULL;
+  res = g_input_stream_read (cstream,
+			     converted, LEFTOVER_SHORT_READ_SIZE,
+			     NULL, &error);
+  g_assert_cmpint (res, ==, LEFTOVER_SHORT_READ_SIZE);
+  total_read += res;
+
+  offset = g_seekable_tell (G_SEEKABLE (mem));
+  g_assert_cmpint (offset, >, LEFTOVER_SHORT_READ_SIZE);
+  g_assert_cmpint (offset, <, LEFTOVER_BUFSIZE);
+
+  /* At this point, @cstream has both a non-empty input_buffer
+   * and a non-empty converted_buffer, which is the case
+   * we want to test.
+   */
+
+  while (TRUE)
+    {
+      error = NULL;
+      res = g_input_stream_read (cstream,
+				 converted + total_read,
+				 LEFTOVER_BUFSIZE - total_read,
+				 NULL, &error);
+      g_assert (res >= 0);
+      if (res == 0)
+	break;
+      total_read += res;
+  }
+
+  g_assert_cmpint (total_read, ==, LEFTOVER_BUFSIZE);
+  g_assert (memcmp (converted, orig, LEFTOVER_BUFSIZE)  == 0);
+
+  g_object_unref (cstream);
+  g_free (orig);
+  g_free (converted);
+  g_object_unref (converter);
+}
+
 #define DATA_LENGTH 1000000
 
 static void
@@ -973,6 +1135,7 @@ main (int   argc,
     g_test_add_data_func (charset_tests[i].path, &charset_tests[i], test_charset);
 
   g_test_add_func ("/converter-stream/pollable", test_converter_pollable);
+  g_test_add_func ("/converter-stream/leftover", test_converter_leftover);
 
   return g_test_run();
 }



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]