]> git.ipfire.org Git - thirdparty/dovecot/core.git/commitdiff
lib: Fixed max_buffer_size handling in istream-chain
authorTimo Sirainen <timo.sirainen@dovecot.fi>
Tue, 31 May 2016 19:19:37 +0000 (22:19 +0300)
committerTimo Sirainen <timo.sirainen@dovecot.fi>
Wed, 1 Jun 2016 10:39:16 +0000 (13:39 +0300)
The default max_buffer_size=256 was wrong in all situations.

We're now assuming that the underlying istreams' max_buffer_size is always
correct. While gluing together two streams we're now allocating enough
memory to hold all of the wanted data (instead of assert-crashing as could
have happened earlier). This means that the max memory usage is actually
the two streams' max_buffer_size summed together. Ideally this would be
fixed to limit the max_buffer_size to maximum of the two, but that would
require further changes.

src/lib/istream-chain.c

index 27fd43778a9e7b659af1d2e7edcd69f8226618fa..89c1bbc59cc3ccdfe70bb1346ecdb704e1840684 100644 (file)
@@ -24,6 +24,7 @@ struct chain_istream {
        struct istream_private istream;
 
        size_t prev_stream_left, prev_skip;
+       bool have_explicit_max_buffer_size;
        
        struct istream_chain chain;
 };
@@ -45,10 +46,9 @@ i_stream_chain_append_internal(struct istream_chain *chain,
                i_stream_ref(stream);   
 
        if (chain->head == NULL && stream != NULL) {
-               if (chain->stream->istream.max_buffer_size == 0) {
-                       chain->stream->istream.max_buffer_size =
-                               stream->real_stream->max_buffer_size;
-               } else {
+               struct chain_istream *cstream = (struct chain_istream *)stream;
+
+               if (cstream->have_explicit_max_buffer_size) {
                        i_stream_set_max_buffer_size(stream,
                                chain->stream->istream.max_buffer_size);
                }
@@ -77,6 +77,7 @@ i_stream_chain_set_max_buffer_size(struct iostream_private *stream,
        struct chain_istream *cstream = (struct chain_istream *)stream;
        struct istream_chain_link *link = cstream->chain.head;
 
+       cstream->have_explicit_max_buffer_size = TRUE;
        cstream->istream.max_buffer_size = max_size;
        while (link != NULL) {
                if (link->stream != NULL)
@@ -106,7 +107,7 @@ static void i_stream_chain_read_next(struct chain_istream *cstream)
        struct istream_chain_link *link = cstream->chain.head;
        struct istream *prev_input;
        const unsigned char *data;
-       size_t data_size, size, cur_data_pos;
+       size_t data_size, cur_data_pos;
 
        i_assert(link != NULL && link->stream != NULL);
        i_assert(link->stream->eof);
@@ -137,17 +138,12 @@ static void i_stream_chain_read_next(struct chain_istream *cstream)
                cstream->prev_stream_left = 0;
        }
 
-       /* we already verified that the data size is less than the
-          maximum buffer size */
        if (data_size > 0) {
-               if (!i_stream_try_alloc(&cstream->istream, data_size, &size))
-                       i_unreached();
-               i_assert(size >= data_size);
+               memcpy(i_stream_alloc(&cstream->istream, data_size),
+                      data, data_size);
+               cstream->istream.pos += data_size;
+               cstream->prev_stream_left += data_size;
        }
-       memcpy(cstream->istream.w_buffer + cstream->istream.pos,
-              data, data_size);
-       cstream->istream.pos += data_size;
-       cstream->prev_stream_left += data_size;
 
        i_stream_skip(prev_input, i_stream_get_data_size(prev_input));
        i_stream_unref(&prev_input);
@@ -190,7 +186,7 @@ static ssize_t i_stream_chain_read(struct istream_private *stream)
        struct chain_istream *cstream = (struct chain_istream *)stream;
        struct istream_chain_link *link = cstream->chain.head;
        const unsigned char *data;
-       size_t size, data_size, cur_data_pos, new_pos;
+       size_t data_size, cur_data_pos, new_pos;
        size_t new_bytes_count;
        ssize_t ret;
 
@@ -251,16 +247,9 @@ static ssize_t i_stream_chain_read(struct istream_private *stream)
                   new data with it. */
                i_assert(data_size > cur_data_pos);
                new_bytes_count = data_size - cur_data_pos;
-               if (!i_stream_try_alloc(stream, new_bytes_count, &size)) {
-                       stream->buffer = stream->w_buffer;
-                       return -2;
-               }
-               stream->buffer = stream->w_buffer;
-
-               if (new_bytes_count > size)
-                       new_bytes_count = size;
-               memcpy(stream->w_buffer + stream->pos,
+               memcpy(i_stream_alloc(stream, new_bytes_count),
                       data + cur_data_pos, new_bytes_count);
+               stream->buffer = stream->w_buffer;
                new_pos = stream->pos + new_bytes_count;
        }
 
@@ -295,7 +284,6 @@ struct istream *i_stream_create_chain(struct istream_chain **chain_r)
 
        cstream = i_new(struct chain_istream, 1);
        cstream->chain.stream = cstream;
-       cstream->istream.max_buffer_size = 256;
 
        cstream->istream.iostream.close = i_stream_chain_close;
        cstream->istream.iostream.destroy = i_stream_chain_destroy;