dovecot-2.2: lib: If two istreams share one parent, i_stream_get...
dovecot at dovecot.org
dovecot at dovecot.org
Thu Jun 19 10:54:13 UTC 2014
details: http://hg.dovecot.org/dovecot-2.2/rev/01061ac25fe1
changeset: 17508:01061ac25fe1
user: Timo Sirainen <tss at iki.fi>
date: Thu Jun 19 13:52:36 2014 +0300
description:
lib: If two istreams share one parent, i_stream_get_data() may have returned corrupted data to another.
This happened only for istreams that used parent's buffer directly instead
of having their own buffer. For now at least we've solved this by truncating
the other stream's buffer so it needs to be read again. Hopefully this is
good enough.
Added also unit test to check this functionality.
diffstat:
src/lib/Makefile.am | 1 +
src/lib/istream.c | 46 ++++++++++++++++++++++++++++++++---
src/lib/test-istream.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++
src/lib/test-lib.c | 1 +
src/lib/test-lib.h | 1 +
5 files changed, 109 insertions(+), 4 deletions(-)
diffs (172 lines):
diff -r 627f2a2ba362 -r 01061ac25fe1 src/lib/Makefile.am
--- a/src/lib/Makefile.am Thu Jun 19 12:38:11 2014 +0300
+++ b/src/lib/Makefile.am Thu Jun 19 13:52:36 2014 +0300
@@ -282,6 +282,7 @@
test-hash-method.c \
test-hex-binary.c \
test-iso8601-date.c \
+ test-istream.c \
test-istream-base64-decoder.c \
test-istream-base64-encoder.c \
test-istream-concat.c \
diff -r 627f2a2ba362 -r 01061ac25fe1 src/lib/istream.c
--- a/src/lib/istream.c Thu Jun 19 12:38:11 2014 +0300
+++ b/src/lib/istream.c Thu Jun 19 13:52:36 2014 +0300
@@ -443,6 +443,26 @@
return stream->real_stream->line_crlf;
}
+static bool i_stream_is_buffer_invalid(const struct istream_private *stream)
+{
+ if (stream->parent == NULL) {
+ /* the buffer can't point to parent, because it doesn't exist */
+ return FALSE;
+ }
+ if (stream->w_buffer != NULL) {
+ /* we can pretty safely assume that the stream is using its
+ own private buffer, so it can never become invalid. */
+ return FALSE;
+ }
+ if (stream->access_counter !=
+ stream->parent->real_stream->access_counter) {
+ /* parent has been modified behind this stream, we can't trust
+ that our buffer is valid */
+ return TRUE;
+ }
+ return i_stream_is_buffer_invalid(stream->parent->real_stream);
+}
+
const unsigned char *
i_stream_get_data(const struct istream *stream, size_t *size_r)
{
@@ -453,6 +473,25 @@
return NULL;
}
+ if (i_stream_is_buffer_invalid(_stream)) {
+ /* This stream may be using parent's buffer directly as
+ _stream->buffer, but the parent stream has already been
+ modified indirectly. This means that the buffer might no
+ longer point to where we assume it points to. So we'll
+ just return the stream as empty until it's read again.
+
+ It's a bit ugly to suddenly drop data from the stream that
+ was already read, but since this happens only with shared
+ parent istreams the caller is hopefully aware enough that
+ something like this might happen. The other solutions would
+ be to a) try to automatically read the data back (but we
+ can't handle errors..) or b) always copy data to stream's
+ own buffer instead of pointing to parent's buffer (but this
+ causes data copying that is nearly always unnecessary). */
+ *size_r = 0;
+ return NULL;
+ }
+
*size_r = _stream->pos - _stream->skip;
return _stream->buffer + _stream->skip;
}
@@ -460,11 +499,10 @@
size_t i_stream_get_data_size(const struct istream *stream)
{
const struct istream_private *_stream = stream->real_stream;
+ size_t size;
- if (_stream->skip >= _stream->pos)
- return 0;
- else
- return _stream->pos - _stream->skip;
+ (void)i_stream_get_data(stream, &size);
+ return size;
}
unsigned char *i_stream_get_modifiable_data(const struct istream *stream,
diff -r 627f2a2ba362 -r 01061ac25fe1 src/lib/test-istream.c
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/src/lib/test-istream.c Thu Jun 19 13:52:36 2014 +0300
@@ -0,0 +1,64 @@
+/* Copyright (c) 2014 Dovecot authors, see the included COPYING file */
+
+#include "test-lib.h"
+#include "istream.h"
+
+static void test_istream_children(void)
+{
+ struct istream *parent, *child1, *child2;
+ const unsigned char *data;
+ size_t size;
+
+ test_begin("istream children");
+
+ parent = test_istream_create_data("123456789", 9);
+ test_istream_set_max_buffer_size(parent, 3);
+
+ child1 = i_stream_create_limit(parent, (uoff_t)-1);
+ child2 = i_stream_create_limit(parent, (uoff_t)-1);
+
+ /* child1 read beginning */
+ test_assert(i_stream_read(child1) == 3);
+ data = i_stream_get_data(child1, &size);
+ test_assert(size == 3 && memcmp(data, "123", 3) == 0);
+ i_stream_skip(child1, 3);
+ /* child1 read middle.. */
+ test_assert(i_stream_read(child1) == 3);
+ data = i_stream_get_data(child1, &size);
+ test_assert(size == 3 && memcmp(data, "456", 3) == 0);
+ /* child2 read beginning.. */
+ test_assert(i_stream_read(child2) == 3);
+ data = i_stream_get_data(child2, &size);
+ test_assert(size == 3 && memcmp(data, "123", 3) == 0);
+ /* child1 check middle again.. the parent has been modified,
+ so it can't return the original data (without some code changes). */
+ data = i_stream_get_data(child1, &size);
+ test_assert(size == 0);
+ i_stream_skip(child1, 3);
+ /* child1 read end */
+ test_assert(i_stream_read(child1) == 3);
+ data = i_stream_get_data(child1, &size);
+ test_assert(size == 3 && memcmp(data, "789", 3) == 0);
+ i_stream_skip(child1, 3);
+ test_assert(i_stream_read(child1) == -1);
+ /* child2 check beginning again.. */
+ data = i_stream_get_data(child2, &size);
+ test_assert(size == 0);
+ i_stream_skip(child2, 3);
+ /* child2 read middle */
+ test_assert(i_stream_read(child2) == 3);
+ data = i_stream_get_data(child2, &size);
+ test_assert(size == 3 && memcmp(data, "456", 3) == 0);
+ i_stream_skip(child2, 3);
+
+ i_stream_destroy(&child1);
+ i_stream_destroy(&child2);
+ i_stream_destroy(&parent);
+
+ test_end();
+}
+
+void test_istream(void)
+{
+ test_istream_children();
+}
diff -r 627f2a2ba362 -r 01061ac25fe1 src/lib/test-lib.c
--- a/src/lib/test-lib.c Thu Jun 19 12:38:11 2014 +0300
+++ b/src/lib/test-lib.c Thu Jun 19 13:52:36 2014 +0300
@@ -17,6 +17,7 @@
test_hash_method,
test_hex_binary,
test_iso8601_date,
+ test_istream,
test_istream_base64_decoder,
test_istream_base64_encoder,
test_istream_concat,
diff -r 627f2a2ba362 -r 01061ac25fe1 src/lib/test-lib.h
--- a/src/lib/test-lib.h Thu Jun 19 12:38:11 2014 +0300
+++ b/src/lib/test-lib.h Thu Jun 19 13:52:36 2014 +0300
@@ -16,6 +16,7 @@
void test_hash_method(void);
void test_hex_binary(void);
void test_iso8601_date(void);
+void test_istream(void);
void test_istream_base64_decoder(void);
void test_istream_base64_encoder(void);
void test_istream_concat(void);
More information about the dovecot-cvs
mailing list