dovecot-2.0: istreams: Reading sometimes returned wrong data, if...
dovecot at dovecot.org
dovecot at dovecot.org
Sun Feb 28 14:13:37 EET 2010
details: http://hg.dovecot.org/dovecot-2.0/rev/de2798fbbae6
changeset: 10810:de2798fbbae6
user: Timo Sirainen <tss at iki.fi>
date: Sun Feb 28 13:41:53 2010 +0200
description:
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.
diffstat:
src/lib-mail/istream-dot.c | 6 --
src/lib-mail/istream-header-filter.c | 83 ++++++++++++++++++++-------
src/lib-mail/test-istream-header-filter.c | 32 ++++++----
src/lib/istream-internal.h | 9 +++
src/lib/istream-tee.c | 16 ++++-
src/lib/istream.c | 55 ++++++++++++------
src/plugins/zlib/istream-bzlib.c | 4 +-
src/plugins/zlib/istream-zlib.c | 4 +-
8 files changed, 140 insertions(+), 69 deletions(-)
diffs (truncated from 479 to 300 lines):
diff -r 6e32dbc4cd8f -r de2798fbbae6 src/lib-mail/istream-dot.c
--- a/src/lib-mail/istream-dot.c Sun Feb 28 13:38:43 2010 +0200
+++ b/src/lib-mail/istream-dot.c Sun Feb 28 13:41:53 2010 +0200
@@ -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 @@
/* 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 @@
}
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 @@
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 -r 6e32dbc4cd8f -r de2798fbbae6 src/lib-mail/istream-header-filter.c
--- a/src/lib-mail/istream-header-filter.c Sun Feb 28 13:38:43 2010 +0200
+++ b/src/lib-mail/istream-header-filter.c Sun Feb 28 13:41:53 2010 +0200
@@ -277,6 +277,7 @@
{
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 @@
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 (v_offset == 0) {
+ /* seeking to beginning of headers. */
+ stream_reset_to(mstream, 0);
+ i_stream_header_filter_seek_to_header(mstream, 0);
+ return;
+ }
- if (mstream->hdr_ctx != NULL) {
- message_parse_header_deinit(&mstream->hdr_ctx);
- mstream->hdr_ctx = NULL;
- }
+ /* 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 @@
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 -r 6e32dbc4cd8f -r de2798fbbae6 src/lib-mail/test-istream-header-filter.c
--- a/src/lib-mail/test-istream-header-filter.c Sun Feb 28 13:38:43 2010 +0200
+++ b/src/lib-mail/test-istream-header-filter.c Sun Feb 28 13:41:53 2010 +0200
@@ -20,43 +20,49 @@
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 -r 6e32dbc4cd8f -r de2798fbbae6 src/lib/istream-internal.h
--- a/src/lib/istream-internal.h Sun Feb 28 13:38:43 2010 +0200
+++ b/src/lib/istream-internal.h Sun Feb 28 13:41:53 2010 +0200
@@ -34,6 +34,15 @@
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 -r 6e32dbc4cd8f -r de2798fbbae6 src/lib/istream-tee.c
--- a/src/lib/istream-tee.c Sun Feb 28 13:38:43 2010 +0200
+++ b/src/lib/istream-tee.c Sun Feb 28 13:41:53 2010 +0200
@@ -39,6 +39,11 @@
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 @@
}
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 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 =
More information about the dovecot-cvs
mailing list