istreams: Reading sometimes returned wrong data, if parent istream had been modified.

We'll now track changes to parent istream using access counters. If parent's
access counter isn't the same as child's, the parent stream is seeked to
expected position.

--HG--
branch : HEAD

diff --git a/src/lib-mail/istream-dot.c b/src/lib-mail/istream-dot.c
index 9b86940125f49beb86f867126d1da8dc57f7b2a6..f5c520040fa49b0b9cee378cb2ba18e9aaf2eed0 100644 (file)
--- a/src/lib-mail/istream-dot.c
+++ b/src/lib-mail/istream-dot.c
@@ -7,8 +7,6 @@
 struct dot_istream {
        struct istream_private istream;
 
-       uoff_t prev_parent_offset;
-
        char pending[3]; /* max. \r\n */
 
        /* how far in string "\r\n.\r" are we */
@@ -130,8 +128,6 @@ static ssize_t i_stream_dot_read(struct istream_private *stream)
 
        /* we have to update stream->pos before reading more data */
        ret1 = i_stream_dot_return(stream, dest, 0);
-
-       i_assert(dstream->prev_parent_offset == stream->parent->v_offset);
        if ((ret = i_stream_dot_read_some(dstream)) <= 0) {
                if (ret1 != 0)
                        return ret1;
@@ -207,7 +203,6 @@ static ssize_t i_stream_dot_read(struct istream_private *stream)
        }
 end:
        i_stream_skip(stream->parent, i);
-       dstream->prev_parent_offset = stream->parent->v_offset;
 
        ret = i_stream_dot_return(stream, dest, 0) + ret1;
        if (ret == 0)
@@ -239,7 +234,6 @@ struct istream *i_stream_create_dot(struct istream *input, bool send_last_lf)
        dstream->state = 2;
        dstream->state_no_cr = TRUE;
        dstream->state_no_lf = TRUE;
-       dstream->prev_parent_offset = input->v_offset;
        return i_stream_create(&dstream->istream, input,
                               i_stream_get_fd(input));
 }

diff --git a/src/lib-mail/istream-header-filter.c b/src/lib-mail/istream-header-filter.c
index bd987017e554a64145ec77860c7422b5c5d1c867..4b1e19c9db1be47bded25220193faa94ba188b0d 100644 (file)
--- a/src/lib-mail/istream-header-filter.c
+++ b/src/lib-mail/istream-header-filter.c
@@ -277,6 +277,7 @@ static ssize_t i_stream_header_filter_read(struct istream_private *stream)
 {
        struct header_filter_istream *mstream =
                (struct header_filter_istream *)stream;
+       uoff_t v_offset;
        ssize_t ret;
 
        if (!mstream->header_read ||
@@ -291,51 +292,84 @@ static ssize_t i_stream_header_filter_read(struct istream_private *stream)
                return -1;
        }
 
-       i_stream_seek(stream->parent, mstream->istream.parent_start_offset +
-                     stream->istream.v_offset -
-                     mstream->header_size.virtual_size +
-                     mstream->header_size.physical_size);
+       v_offset = stream->parent_start_offset + stream->istream.v_offset -
+               mstream->header_size.virtual_size +
+               mstream->header_size.physical_size;
+       i_stream_seek(stream->parent, v_offset);
        return i_stream_read_copy_from_parent(&stream->istream);
 }
 
-static void parse_header(struct header_filter_istream *mstream)
+static void
+i_stream_header_filter_seek_to_header(struct header_filter_istream *mstream,
+                                     uoff_t v_offset)
+{
+       i_stream_seek(mstream->istream.parent,
+                     mstream->istream.parent_start_offset);
+       mstream->istream.parent_expected_offset =
+               mstream->istream.parent_start_offset;
+       mstream->istream.access_counter =
+               mstream->istream.parent->real_stream->access_counter;
+
+       if (mstream->hdr_ctx != NULL)
+               message_parse_header_deinit(&mstream->hdr_ctx);
+       mstream->skip_count = v_offset;
+       mstream->cur_line = 0;
+       mstream->header_read = FALSE;
+       mstream->seen_eoh = FALSE;
+}
+
+static void skip_header(struct header_filter_istream *mstream)
 {
        size_t pos;
 
-       while (!mstream->header_read) {
-               if (i_stream_header_filter_read(&mstream->istream) == -1)
-                       break;
+       if (mstream->header_read)
+               return;
+
+       if (mstream->istream.access_counter !=
+           mstream->istream.parent->real_stream->access_counter) {
+               /* need to re-parse headers */
+               i_stream_header_filter_seek_to_header(mstream, 0);
+       }
 
+       while (!mstream->header_read &&
+              i_stream_read(&mstream->istream.istream) != -1) {
                (void)i_stream_get_data(&mstream->istream.istream, &pos);
                i_stream_skip(&mstream->istream.istream, pos);
        }
 }
 
+static void
+stream_reset_to(struct header_filter_istream *mstream, uoff_t v_offset)
+{
+       mstream->istream.istream.v_offset = v_offset;
+       mstream->istream.skip = mstream->istream.pos = 0;
+       mstream->istream.buffer = NULL;
+       buffer_set_used_size(mstream->hdr_buf, 0);
+}
+
 static void i_stream_header_filter_seek(struct istream_private *stream,
                                        uoff_t v_offset, bool mark ATTR_UNUSED)
 {
        struct header_filter_istream *mstream =
                (struct header_filter_istream *)stream;
 
-       parse_header(mstream);
-       stream->istream.v_offset = v_offset;
-       stream->skip = stream->pos = 0;
-       stream->buffer = NULL;
-       buffer_set_used_size(mstream->hdr_buf, 0);
-
-       if (mstream->hdr_ctx != NULL) {
-               message_parse_header_deinit(&mstream->hdr_ctx);
-               mstream->hdr_ctx = NULL;
+       if (v_offset == 0) {
+               /* seeking to beginning of headers. */
+               stream_reset_to(mstream, 0);
+               i_stream_header_filter_seek_to_header(mstream, 0);
+               return;
        }
 
+       /* if we haven't parsed the whole header yet, we don't know if we
+          want to seek inside header or body. so make sure we've parsed the
+          header. */
+       skip_header(mstream);
+       stream_reset_to(mstream, v_offset);
+
        if (v_offset < mstream->header_size.virtual_size) {
                /* seek into headers. we'll have to re-parse them, use
                   skip_count to set the wanted position */
-               i_stream_seek(stream->parent, stream->parent_start_offset);
-               mstream->skip_count = v_offset;
-               mstream->cur_line = 0;
-               mstream->header_read = FALSE;
-               mstream->seen_eoh = FALSE;
+               i_stream_header_filter_seek_to_header(mstream, v_offset);
        } else {
                /* body */
                v_offset += mstream->header_size.physical_size -
@@ -357,17 +391,20 @@ i_stream_header_filter_stat(struct istream_private *stream, bool exact)
        struct header_filter_istream *mstream =
                (struct header_filter_istream *)stream;
        const struct stat *st;
+       uoff_t old_offset;
 
        st = i_stream_stat(stream->parent, exact);
        if (st == NULL || st->st_size == -1 || !exact)
                return st;
 
-       parse_header(mstream);
+       old_offset = stream->istream.v_offset;
+       skip_header(mstream);
 
        stream->statbuf = *st;
        stream->statbuf.st_size -=
                (off_t)mstream->header_size.physical_size -
                (off_t)mstream->header_size.virtual_size;
+       i_stream_seek(&stream->istream, old_offset);
        return &stream->statbuf;
 }
 

diff --git a/src/lib-mail/test-istream-header-filter.c b/src/lib-mail/test-istream-header-filter.c
index 6b0a11712699d43634c884e6b82e2a5fe6213f58..1e3ed031911f3328d43ac3483a07cd20f2c785ed 100644 (file)
--- a/src/lib-mail/test-istream-header-filter.c
+++ b/src/lib-mail/test-istream-header-filter.c
@@ -20,43 +20,49 @@ static void test_istream_filter(void)
        static const char *exclude_headers[] = { "To", NULL };
        const char *input = "From: foo\nFrom: abc\nTo: bar\n\nhello world\n";
        const char *output = "From: abc\n\nhello world\n";
-       struct istream *istream, *filter;
+       struct istream *istream, *filter, *filter2;
        unsigned int i, input_len = strlen(input);
        unsigned int output_len = strlen(output);
        const unsigned char *data;
        size_t size;
        ssize_t ret;
-       bool success = TRUE;
 
+       test_begin("i_stream_create_header_filter()");
        istream = test_istream_create(input);
        filter = i_stream_create_header_filter(istream,
                                               HEADER_FILTER_EXCLUDE |
                                               HEADER_FILTER_NO_CR,
                                               exclude_headers, 1,
                                               filter_callback, NULL);
-       for (i = 1; i <= input_len; i++) {
+       filter2 = i_stream_create_header_filter(filter,
+                                               HEADER_FILTER_EXCLUDE |
+                                               HEADER_FILTER_NO_CR,
+                                               exclude_headers, 1,
+                                               null_header_filter_callback, NULL);
+       i_stream_unref(&filter);
+       filter = filter2;
+
+       for (i = 1; i < input_len; i++) {
                test_istream_set_size(istream, i);
-               ret = i_stream_read(filter);
-               if (ret < 0) {
-                       success = FALSE;
-                       break;
-               }
+               test_assert(i_stream_read(filter) >= 0);
        }
+       test_istream_set_size(istream, input_len);
+       test_assert(i_stream_read(filter) > 0);
+       test_assert(i_stream_read(filter) == -1);
+
        data = i_stream_get_data(filter, &size);
-       if (size != output_len || memcmp(data, output, size) != 0)
-               success = FALSE;
+       test_assert(size == output_len && memcmp(data, output, size) == 0);
 
        i_stream_skip(filter, size);
        i_stream_seek(filter, 0);
        while ((ret = i_stream_read(filter)) > 0) ;
        data = i_stream_get_data(filter, &size);
-       if (size != output_len || memcmp(data, output, size) != 0)
-               success = FALSE;
+       test_assert(size == output_len && memcmp(data, output, size) == 0);
 
        i_stream_unref(&filter);
        i_stream_unref(&istream);
 
-       test_out("i_stream_create_header_filter()", success);
+       test_end();
 }
 
 int main(void)

diff --git a/src/lib/istream-internal.h b/src/lib/istream-internal.h
index 1085b17fb2fe73bd88cb99d38f51c39eae9cee02..d245c6a9ed60bdcea638983af3e19fc8f6196bc3 100644 (file)
--- a/src/lib/istream-internal.h
+++ b/src/lib/istream-internal.h
@@ -34,6 +34,15 @@ struct istream_private {
        struct istream *parent; /* for filter streams */
        uoff_t parent_start_offset;
 
+       /* parent stream's expected offset is kept here. i_stream_read()
+          always seeks parent stream to here before calling read(). */
+       uoff_t parent_expected_offset;
+
+       /* increased every time the stream is changed (e.g. seek, read).
+          this way streams can check if their parent streams have been
+          accessed behind them. */
+       unsigned int access_counter;
+
        string_t *line_str; /* for i_stream_next_line() if w_buffer == NULL */
        unsigned int return_nolf_line:1;
 };

diff --git a/src/lib/istream-tee.c b/src/lib/istream-tee.c
index a2c11741f057340700087bea2e42fab1fc08714d..490fb8606019a2b0e748a7875410c58b06bd7060 100644 (file)
--- a/src/lib/istream-tee.c
+++ b/src/lib/istream-tee.c
@@ -39,6 +39,11 @@ static void tee_streams_update_buffer(struct tee_istream *tee)
                        tee->input->v_offset;
                i_assert(tstream->istream.skip + old_used <= size);
                tstream->istream.pos = tstream->istream.skip + old_used;
+
+               tstream->istream.parent_expected_offset =
+                       tee->input->v_offset;
+               tstream->istream.access_counter =
+                       tee->input->real_stream->access_counter;
        }
 }
 
@@ -84,6 +89,7 @@ static void i_stream_tee_destroy(struct iostream_private *stream)
        }
 
        if (tee->children == NULL) {
+               /* last child. the tee is now destroyed */
                i_assert(tee->input->v_offset <= tee->max_read_offset);
                i_stream_skip(tee->input,
                              tee->max_read_offset - tee->input->v_offset);
@@ -195,12 +201,12 @@ struct tee_istream *tee_i_stream_create(struct istream *input)
 struct istream *tee_i_stream_create_child(struct tee_istream *tee)
 {
        struct tee_child_istream *tstream;
+       struct istream *ret, *input = tee->input;
 
        tstream = i_new(struct tee_child_istream, 1);
        tstream->tee = tee;
 
-       tstream->istream.max_buffer_size =
-               tee->input->real_stream->max_buffer_size;
+       tstream->istream.max_buffer_size = input->real_stream->max_buffer_size;
        tstream->istream.iostream.close = i_stream_tee_close;
        tstream->istream.iostream.destroy = i_stream_tee_destroy;
        tstream->istream.iostream.set_max_buffer_size =
@@ -213,8 +219,10 @@ struct istream *tee_i_stream_create_child(struct tee_istream *tee)
        tstream->next = tee->children;
        tee->children = tstream;
 
-       return i_stream_create(&tstream->istream, NULL,
-                              i_stream_get_fd(tee->input));
+       ret = i_stream_create(&tstream->istream, input, i_stream_get_fd(input));
+       /* we keep the reference in tee stream, no need for extra references */
+       i_stream_unref(&input);
+       return ret;
 }
 
 bool tee_i_stream_child_is_waiting(struct istream *input)

diff --git a/src/lib/istream.c b/src/lib/istream.c
index 6da8a426ebe14f9924639babea8d6fb222fced02..601b9c7e9084e1fd38442943501d538696bd65aa 100644 (file)
--- a/src/lib/istream.c
+++ b/src/lib/istream.c
@@ -69,6 +69,17 @@ void i_stream_set_return_partial_line(struct istream *stream, bool set)
        stream->real_stream->return_nolf_line = set;
 }
 
+static void i_stream_update(struct istream_private *stream)
+{
+       if (stream->parent == NULL)
+               stream->access_counter++;
+       else {
+               stream->access_counter =
+                       stream->parent->real_stream->access_counter;
+               stream->parent_expected_offset = stream->parent->v_offset;
+       }
+}
+
 ssize_t i_stream_read(struct istream *stream)
 {
        struct istream_private *_stream = stream->real_stream;
@@ -81,6 +92,9 @@ ssize_t i_stream_read(struct istream *stream)
        stream->eof = FALSE;
        stream->stream_errno = 0;
 
+       if (_stream->parent != NULL)
+               i_stream_seek(_stream->parent, _stream->parent_expected_offset);
+
        old_size = _stream->pos - _stream->skip;
        ret = _stream->read(_stream);
        switch (ret) {
@@ -104,6 +118,8 @@ ssize_t i_stream_read(struct istream *stream)
                i_assert((size_t)ret+old_size == _stream->pos - _stream->skip);
                break;
        }
+
+       i_stream_update(_stream);
        return ret;
 }
 
@@ -164,21 +180,17 @@ void i_stream_skip(struct istream *stream, uoff_t count)
        _stream->seek(_stream, stream->v_offset + count, FALSE);
 }
 
-static bool i_stream_can_optimize_seek(struct istream *stream)
+static bool i_stream_can_optimize_seek(struct istream_private *stream)
 {
-       uoff_t expected_offset;
-
-       if (stream->real_stream->parent == NULL)
+       if (stream->parent == NULL)
                return TRUE;
 
-       /* use the fast route only if the parent stream is at the
-          expected offset */
-       expected_offset = stream->real_stream->parent_start_offset +
-               stream->v_offset - stream->real_stream->skip;
-       if (stream->real_stream->parent->v_offset != expected_offset)
+       /* use the fast route only if the parent stream hasn't been changed */
+       if (stream->access_counter !=
+           stream->parent->real_stream->access_counter)
                return FALSE;
 
-       return i_stream_can_optimize_seek(stream->real_stream->parent);
+       return i_stream_can_optimize_seek(stream->parent->real_stream);
 }
 
 void i_stream_seek(struct istream *stream, uoff_t v_offset)
@@ -186,16 +198,16 @@ void i_stream_seek(struct istream *stream, uoff_t v_offset)
        struct istream_private *_stream = stream->real_stream;
 
        if (v_offset >= stream->v_offset &&
-           i_stream_can_optimize_seek(stream)) {
+           i_stream_can_optimize_seek(_stream))
                i_stream_skip(stream, v_offset - stream->v_offset);
-               return;
-       }
-
-       if (unlikely(stream->closed))
-               return;
+       else {
+               if (unlikely(stream->closed))
+                       return;
 
-       stream->eof = FALSE;
-       _stream->seek(_stream, v_offset, FALSE);
+               stream->eof = FALSE;
+               _stream->seek(_stream, v_offset, FALSE);
+       }
+       i_stream_update(_stream);
 }
 
 void i_stream_seek_mark(struct istream *stream, uoff_t v_offset)
@@ -207,6 +219,7 @@ void i_stream_seek_mark(struct istream *stream, uoff_t v_offset)
 
        stream->eof = FALSE;
        _stream->seek(_stream, v_offset, TRUE);
+       i_stream_update(_stream);
 }
 
 void i_stream_sync(struct istream *stream)
@@ -216,8 +229,10 @@ void i_stream_sync(struct istream *stream)
        if (unlikely(stream->closed))
                return;
 
-       if (_stream->sync != NULL)
+       if (_stream->sync != NULL) {
                _stream->sync(_stream);
+               i_stream_update(_stream);
+       }
 }
 
 const struct stat *i_stream_stat(struct istream *stream, bool exact)
@@ -549,8 +564,10 @@ i_stream_create(struct istream_private *_stream, struct istream *parent, int fd)
 {
        _stream->fd = fd;
        if (parent != NULL) {
+               _stream->access_counter = parent->real_stream->access_counter;
                _stream->parent = parent;
                _stream->parent_start_offset = parent->v_offset;
+               _stream->parent_expected_offset = parent->v_offset;
                _stream->abs_start_offset = parent->v_offset +
                        parent->real_stream->abs_start_offset;
                i_stream_ref(parent);

diff --git a/src/plugins/zlib/istream-bzlib.c b/src/plugins/zlib/istream-bzlib.c
index b057f6862bb607758ee8e015851a8f1f5e3a66cc..f7ca31cfdb50dd1ee174f70c2450ae49a6694aa2 100644 (file)
--- a/src/plugins/zlib/istream-bzlib.c
+++ b/src/plugins/zlib/istream-bzlib.c
@@ -193,7 +193,7 @@ i_stream_bzlib_seek(struct istream_private *stream, uoff_t v_offset, bool mark)
                        }
 
                        i_stream_skip(&stream->istream, avail);
-               } while (i_stream_bzlib_read(stream) >= 0);
+               } while (i_stream_read(&stream->istream) >= 0);
 
                if (stream->istream.v_offset != v_offset) {
                        /* some failure, we've broken it */
@@ -236,7 +236,7 @@ i_stream_bzlib_stat(struct istream_private *stream, bool exact)
                do {
                        (void)i_stream_get_data(&stream->istream, &size);
                        i_stream_skip(&stream->istream, size);
-               } while (i_stream_bzlib_read(stream) > 0);
+               } while (i_stream_read(&stream->istream) > 0);
 
                i_stream_seek(&stream->istream, old_offset);
                if (zstream->eof_offset == (uoff_t)-1)

diff --git a/src/plugins/zlib/istream-zlib.c b/src/plugins/zlib/istream-zlib.c
index b3e7605e42cc362bf957ac0f889ea840b873dde0..b78a9152921a3620bcad3830a8df3a0af225c333 100644 (file)
--- a/src/plugins/zlib/istream-zlib.c
+++ b/src/plugins/zlib/istream-zlib.c
@@ -327,7 +327,7 @@ i_stream_zlib_seek(struct istream_private *stream, uoff_t v_offset, bool mark)
                        }
 
                        i_stream_skip(&stream->istream, avail);
-               } while (i_stream_zlib_read(stream) >= 0);
+               } while (i_stream_read(&stream->istream) >= 0);
 
                if (stream->istream.v_offset != v_offset) {
                        /* some failure, we've broken it */
@@ -370,7 +370,7 @@ i_stream_zlib_stat(struct istream_private *stream, bool exact)
                do {
                        (void)i_stream_get_data(&stream->istream, &size);
                        i_stream_skip(&stream->istream, size);
-               } while (i_stream_zlib_read(stream) > 0);
+               } while (i_stream_read(&stream->istream) > 0);
 
                i_stream_seek(&stream->istream, old_offset);
                if (zstream->eof_offset == (uoff_t)-1)