lib: When closing istream-chain, make sure parent stream is seeked to…

… correct offset.

We were only seeking it earlier if it ended at EOF.

src/lib/Makefile.am

@@ -312,6 +312,7 @@ test_lib_SOURCES = \
 	test-istream.c \
 	test-istream-base64-decoder.c \
 	test-istream-base64-encoder.c \
+	test-istream-chain.c \
 	test-istream-concat.c \
 	test-istream-crlf.c \
 	test-istream-failure-at.c \

src/lib/istream-chain.c

@@ -153,19 +153,11 @@ static void i_stream_chain_read_next(struct chain_istream *cstream)
 	i_stream_unref(&prev_input);
 }
 
-static ssize_t i_stream_chain_read(struct istream_private *stream)
+static bool i_stream_chain_skip(struct chain_istream *cstream)
 {
-	struct chain_istream *cstream = (struct chain_istream *)stream;
+	struct istream_private *stream = &cstream->istream;
 	struct istream_chain_link *link = cstream->chain.head;
-	const unsigned char *data;
-	size_t size, data_size, cur_data_pos, new_pos, bytes_skipped;
-	size_t new_bytes_count;
-	ssize_t ret;
-
-	if (link != NULL && link->eof) {
-		stream->istream.eof = TRUE;
-		return -1;
-	}
+	size_t bytes_skipped;
 
 	i_assert(stream->skip >= cstream->prev_skip);
 	bytes_skipped = stream->skip - cstream->prev_skip;
@@ -185,12 +177,30 @@ static ssize_t i_stream_chain_read(struct istream_private *stream)
 	stream->skip -= bytes_skipped;
 	stream->buffer += bytes_skipped;
 	cstream->prev_skip = stream->skip;
-
-	if (link == NULL) {
+	if (link == NULL || link->eof) {
 		i_assert(bytes_skipped == 0);
-		return 0;
+		return FALSE;
 	}
 	i_stream_skip(link->stream, bytes_skipped);
+	return TRUE;
+}
+
+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 new_bytes_count;
+	ssize_t ret;
+
+	if (link != NULL && link->eof) {
+		stream->istream.eof = TRUE;
+		return -1;
+	}
+
+	if (!i_stream_chain_skip(cstream))
+		return 0;
 
 	i_assert(stream->pos >= stream->skip + cstream->prev_stream_left);
 	cur_data_pos = stream->pos - (stream->skip + cstream->prev_stream_left);
@@ -260,6 +270,24 @@ static ssize_t i_stream_chain_read(struct istream_private *stream)
 	return ret;
 }
 
+static void i_stream_chain_close(struct iostream_private *stream,
+				 bool close_parent)
+{
+	struct chain_istream *cstream = (struct chain_istream *)stream;
+
+	/* seek to the correct position in parent stream in case it didn't
+	   end with EOF */
+	(void)i_stream_chain_skip(cstream);
+
+	if (close_parent) {
+		struct istream_chain_link *link = cstream->chain.head;
+		while (link != NULL) {
+			i_stream_close(link->stream);
+			link = link->next;
+		}
+	}
+}
+
 struct istream *i_stream_create_chain(struct istream_chain **chain_r)
 {
 	struct chain_istream *cstream;
@@ -268,6 +296,7 @@ struct istream *i_stream_create_chain(struct istream_chain **chain_r)
 	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;
 	cstream->istream.iostream.set_max_buffer_size =
 		i_stream_chain_set_max_buffer_size;

src/lib/test-istream-chain.c

@@ -0,0 +1,87 @@
+/* Copyright (c) 2016 Dovecot authors, see the included COPYING file */
+
+#include "test-lib.h"
+#include "istream-private.h"
+#include "istream-chain.h"
+
+static void test_istream_chain_basic(void)
+{
+	struct istream *input, *test_input, *test_input2;
+	struct istream_chain *chain;
+	const unsigned char *data;
+	size_t size;
+
+	test_begin("istream chain");
+
+	test_input = test_istream_create("stream1");
+	test_input2 = test_istream_create("STREAM2");
+
+	input = i_stream_create_chain(&chain);
+	/* no input */
+	test_assert(i_stream_read(input) == 0);
+	/* stream1 input */
+	i_stream_chain_append(chain, test_input);
+	test_assert(i_stream_read(input) == 7);
+	data = i_stream_get_data(input, &size);
+	test_assert(size == 7 && memcmp(data, "stream1", 7) == 0);
+	test_assert(i_stream_read(input) == 0);
+	data = i_stream_get_data(input, &size);
+	test_assert(size == 7 && memcmp(data, "stream1", 7) == 0);
+	/* STREAM2 input */
+	i_stream_chain_append(chain, test_input2);
+	test_assert(i_stream_read(input) == 7);
+	data = i_stream_get_data(input, &size);
+	test_assert(size == 14 && memcmp(data, "stream1STREAM2", 14) == 0);
+	test_assert(i_stream_read(input) == 0);
+	data = i_stream_get_data(input, &size);
+	test_assert(size == 14 && memcmp(data, "stream1STREAM2", 14) == 0);
+	/* EOF */
+	i_stream_chain_append_eof(chain);
+	test_assert(i_stream_read(input) == -1 &&
+		    input->eof && input->stream_errno == 0);
+	data = i_stream_get_data(input, &size);
+	test_assert(size == 14 && memcmp(data, "stream1STREAM2", 14) == 0);
+
+	i_stream_unref(&input);
+
+	test_assert(i_stream_is_eof(test_input));
+	test_assert(i_stream_is_eof(test_input2));
+
+	i_stream_unref(&test_input);
+	i_stream_unref(&test_input2);
+	test_end();
+}
+
+static void test_istream_chain_early_end(void)
+{
+	struct istream *input, *test_input;
+	struct istream_chain *chain;
+
+	test_begin("istream chain early end");
+
+	test_input = test_istream_create("string");
+	test_istream_set_size(test_input, 3);
+	test_istream_set_allow_eof(test_input, FALSE);
+
+	input = i_stream_create_chain(&chain);
+	i_stream_chain_append(chain, test_input);
+	test_assert(i_stream_read(input) == 3);
+	test_istream_set_size(test_input, 5);
+	test_assert(i_stream_read(input) == 2);
+	/* with current implementation we could skip less than 5 and have
+	   v_offset<5, but I don't think that can work in all situations.
+	   the normal case is anyway that we'll read everything up until some
+	   point and skip over all the data up to there. */
+	i_stream_skip(input, 5);
+	i_stream_unref(&input);
+
+	test_assert(test_input->v_offset == 5);
+	i_stream_unref(&test_input);
+	test_end();
+}
+
+void test_istream_chain(void)
+{
+	test_istream_chain_basic();
+	test_istream_chain_early_end();
+}

src/lib/test-lib.c

@@ -25,6 +25,7 @@ int main(void)
 		test_istream,
 		test_istream_base64_decoder,
 		test_istream_base64_encoder,
+		test_istream_chain,
 		test_istream_concat,
 		test_istream_crlf,
 		test_istream_failure_at,

src/lib/test-lib.h

@@ -26,6 +26,7 @@ void test_iso8601_date(void);
 void test_istream(void);
 void test_istream_base64_decoder(void);
 void test_istream_base64_encoder(void);
+void test_istream_chain(void);
 void test_istream_concat(void);
 void test_istream_crlf(void);
 void test_istream_failure_at(void);