From: Stephan Bosch <stephan.bosch@open-xchange.com>
Date: Mon, 3 May 2021 21:54:26 +0000 (+0200)
Subject: lib-program-client: program-client-remote - Fix result parsing in the istream.
X-Git-Tag: 2.3.16~228
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=1666e897f3f199a541764c1a5f5468bb86d2c7bb;p=thirdparty%2Fdovecot%2Fcore.git

lib-program-client: program-client-remote - Fix result parsing in the istream.

This fixes internal errors occurring when the parent stream is fully read before
EOF is signaled. This was caused by the fact that the final bytes were not
always reserved properly for parsing, so that these ended up being exposed to
and eaten by the application. At the end, the result parsing code would be faced
with no data, thereby causing an internal error to be returned by the calling
application.

This came to light as CI tests involving Sieve extprograms failed irregularly.
This problem only occurs when a (slight) delay exists between the last data
being sent and the connection being closed by the script service, making this
hard to reproduce. It typically occurred only under high server load.
---

diff --git a/src/lib-program-client/program-client-remote.c b/src/lib-program-client/program-client-remote.c
index 16cf0d1f22..54b5d5a32a 100644
--- a/src/lib-program-client/program-client-remote.c
+++ b/src/lib-program-client/program-client-remote.c
@@ -35,6 +35,8 @@ struct program_client_istream {
 	struct stat statbuf;
 
 	struct program_client *client;
+
+	bool parsed_result:1;
 };
 
 static void program_client_istream_destroy(struct iostream_private *stream)
@@ -51,6 +53,10 @@ program_client_istream_parse_result(struct program_client_istream *scstream,
 {
 	struct istream_private *stream = &scstream->istream;
 
+	if (scstream->parsed_result)
+		return;
+	scstream->parsed_result = TRUE;
+
 	if (stream->buffer == NULL || pos < 2 ||
 	    stream->buffer[pos - 1] != '\n') {
 		if (pos == 0) {
@@ -105,6 +111,11 @@ static ssize_t program_client_istream_read(struct istream_private *stream)
 
 	stream->buffer = i_stream_get_data(stream->parent, &pos);
 
+	if (stream->parent->eof) {
+		/* Check return code at EOF */
+		program_client_istream_parse_result(scstream, pos);
+	}
+
 	reserved = 0;
 	if (stream->buffer != NULL && pos >= 1) {
 		/* Retain/hide potential return code at end of buffer */
@@ -113,6 +124,7 @@ static ssize_t program_client_istream_read(struct istream_private *stream)
 	}
 
 	if (stream->parent->eof) {
+		i_assert(scstream->parsed_result);
 		if (pos == 0)
 			i_stream_skip(stream->parent, reserved);
 		stream->istream.eof = TRUE;
@@ -130,8 +142,6 @@ static ssize_t program_client_istream_read(struct istream_private *stream)
 			}
 			if (ret < 0 && stream->istream.stream_errno != 0)
 				break;
-			if (ret == 0)
-				break;
 
 			if (stream->parent->eof) {
 				/* Check return code at EOF */
@@ -139,24 +149,32 @@ static ssize_t program_client_istream_read(struct istream_private *stream)
 					scstream, pos);
 			}
 
+			ssize_t reserve_mod = 0;
 			if (stream->buffer != NULL && pos >= 1) {
 				/* Retain/hide potential return code at end of
 				   buffer */
 				size_t old_reserved = reserved;
-				ssize_t reserve_mod;
 
 				reserved = (stream->buffer[pos - 1] == '\n' &&
 					    pos > 1 ? 2 : 1);
 				reserve_mod = reserved - old_reserved;
 				pos -= reserved;
-
-				if (ret >= reserve_mod)
-					ret -= reserve_mod;
+			}
+			if (ret == 0) {
+				/* Parent already blocked, but we had to update
+				   pos first, to make sure reserved bytes are
+				   not visible to application. */
+				break;
+			}
+			if (ret >= reserve_mod) {
+				/* Subtract additional reserved bytes */
+				ret -= reserve_mod;
 			}
 
 			if (ret <= 0 && stream->parent->eof) {
 				/* Parent EOF and not more data to return;
 				   EOF here as well */
+				i_assert(scstream->parsed_result);
 				if (pos == 0)
 					i_stream_skip(stream->parent, reserved);
 				stream->istream.eof = TRUE;