Merge branch 'jk/sideband-more-error-checking'

The code to detect premature EOF in the sideband demultiplexer has
been cleaned up.

* jk/sideband-more-error-checking:
  sideband: diagnose more sideband anomalies

diff --git a/pkt-line.c b/pkt-line.c
index 657a702927dbbdad837ec022e33cf30745a6e4ce..d633005ef746ad1eb7445cb75401e0c3f55a2312 100644 (file)
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -461,9 +461,11 @@ int recv_sideband(const char *me, int in_stream, int out)
        enum sideband_type sideband_type;
 
        while (1) {
-               len = packet_read(in_stream, NULL, NULL, buf, LARGE_PACKET_MAX,
-                                 0);
-               if (!demultiplex_sideband(me, buf, len, 0, &scratch,
+               int status = packet_read_with_status(in_stream, NULL, NULL,
+                                                    buf, LARGE_PACKET_MAX,
+                                                    &len,
+                                                    PACKET_READ_GENTLE_ON_EOF);
+               if (!demultiplex_sideband(me, status, buf, len, 0, &scratch,
                                          &sideband_type))
                        continue;
                switch (sideband_type) {
@@ -520,9 +522,9 @@ enum packet_read_status packet_reader_read(struct packet_reader *reader)
                                                         reader->options);
                if (!reader->use_sideband)
                        break;
-               if (demultiplex_sideband(reader->me, reader->buffer,
-                                        reader->pktlen, 1, &scratch,
-                                        &sideband_type))
+               if (demultiplex_sideband(reader->me, reader->status,
+                                        reader->buffer, reader->pktlen, 1,
+                                        &scratch, &sideband_type))
                        break;
        }
 

diff --git a/sideband.c b/sideband.c
index a5405b9aaa6826d8ba30a0bea812fbdc9c80bc01..6f9e02673201ac111acd7f69e0388282de834ed5 100644 (file)
--- a/sideband.c
+++ b/sideband.c
@@ -3,6 +3,7 @@
 #include "config.h"
 #include "sideband.h"
 #include "help.h"
+#include "pkt-line.h"
 
 struct keyword_entry {
        /*
@@ -114,7 +115,8 @@ static void maybe_colorize_sideband(struct strbuf *dest, const char *src, int n)
 #define ANSI_SUFFIX "\033[K"
 #define DUMB_SUFFIX "        "
 
-int demultiplex_sideband(const char *me, char *buf, int len,
+int demultiplex_sideband(const char *me, int status,
+                        char *buf, int len,
                         int die_on_error,
                         struct strbuf *scratch,
                         enum sideband_type *sideband_type)
@@ -130,17 +132,30 @@ int demultiplex_sideband(const char *me, char *buf, int len,
                        suffix = DUMB_SUFFIX;
        }
 
-       if (len == 0) {
-               *sideband_type = SIDEBAND_FLUSH;
-               goto cleanup;
-       }
-       if (len < 1) {
+       if (status == PACKET_READ_EOF) {
                strbuf_addf(scratch,
-                           "%s%s: protocol error: no band designator",
+                           "%s%s: unexpected disconnect while reading sideband packet",
                            scratch->len ? "\n" : "", me);
                *sideband_type = SIDEBAND_PROTOCOL_ERROR;
                goto cleanup;
        }
+
+       if (len < 0)
+               BUG("negative length on non-eof packet read");
+
+       if (len == 0) {
+               if (status == PACKET_READ_NORMAL) {
+                       strbuf_addf(scratch,
+                                   "%s%s: protocol error: missing sideband designator",
+                                   scratch->len ? "\n" : "", me);
+                       *sideband_type = SIDEBAND_PROTOCOL_ERROR;
+               } else {
+                       /* covers flush, delim, etc */
+                       *sideband_type = SIDEBAND_FLUSH;
+               }
+               goto cleanup;
+       }
+
        band = buf[0] & 0xff;
        buf[len] = '\0';
        len--;

diff --git a/sideband.h b/sideband.h
index 227740a58e58bd25c4625bc1963106542bd99779..5a25331be55d30dbc6b4d777da3cf015118dcf3d 100644 (file)
--- a/sideband.h
+++ b/sideband.h
@@ -18,8 +18,12 @@ enum sideband_type {
  *
  * scratch must be a struct strbuf allocated by the caller. It is used to store
  * progress messages split across multiple packets.
+ *
+ * The "status" parameter is a pkt-line response as returned by
+ * packet_read_with_status() (e.g., PACKET_READ_NORMAL).
  */
-int demultiplex_sideband(const char *me, char *buf, int len,
+int demultiplex_sideband(const char *me, int status,
+                        char *buf, int len,
                         int die_on_error,
                         struct strbuf *scratch,
                         enum sideband_type *sideband_type);

diff --git a/t/t0070-fundamental.sh b/t/t0070-fundamental.sh
index 357201640a70e11470c6cd2a3b12d0ef013d32dc..8d59905ef099bf0ab46d0681568dd17d7ed85d98 100755 (executable)
--- a/t/t0070-fundamental.sh
+++ b/t/t0070-fundamental.sh
@@ -40,4 +40,16 @@ test_expect_success 'incomplete sideband messages are reassembled' '
        grep "Hello, world" err
 '
 
+test_expect_success 'eof on sideband message is reported' '
+       printf 1234 >input &&
+       test-tool pkt-line receive-sideband <input 2>err &&
+       test_i18ngrep "unexpected disconnect" err
+'
+
+test_expect_success 'missing sideband designator is reported' '
+       printf 0004 >input &&
+       test-tool pkt-line receive-sideband <input 2>err &&
+       test_i18ngrep "missing sideband" err
+'
+
 test_done