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 16cf0d1f2283a31401c87a322a7cf4fbbc25bd48..54b5d5a32a341248026c4c2dae8e90c3c85ea14c 100644 (file)
--- 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;