Merge branch 'jx/sideband-chomp-newline-fix' into maint-2.43

Sideband demultiplexer fixes.

* jx/sideband-chomp-newline-fix:
  pkt-line: do not chomp newlines for sideband messages
  pkt-line: memorize sideband fragment in reader
  test-pkt-line: add option parser for unpack-sideband

diff --git a/pkt-line.c b/pkt-line.c
index 236dd3a3ee1522331595959e5b68b4f00c9c6275..24479eae4dbe2a44b15a10f86b15bfc6f865bb34 100644 (file)
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -463,8 +463,32 @@ enum packet_read_status packet_read_with_status(int fd, char **src_buffer,
        }
 
        if ((options & PACKET_READ_CHOMP_NEWLINE) &&
-           len && buffer[len-1] == '\n')
-               len--;
+           len && buffer[len-1] == '\n') {
+               if (options & PACKET_READ_USE_SIDEBAND) {
+                       int band = *buffer & 0xff;
+                       switch (band) {
+                       case 1:
+                               /* Chomp newline for payload */
+                               len--;
+                               break;
+                       case 2:
+                       case 3:
+                               /*
+                                * Do not chomp newline for progress and error
+                                * message.
+                                */
+                               break;
+                       default:
+                               /*
+                                * Bad sideband, let's leave it to
+                                * demultiplex_sideband() to catch this error.
+                                */
+                               break;
+                       }
+               } else {
+                       len--;
+               }
+       }
 
        buffer[len] = 0;
        if (options & PACKET_READ_REDACT_URI_PATH &&
@@ -593,17 +617,19 @@ void packet_reader_init(struct packet_reader *reader, int fd,
        reader->options = options;
        reader->me = "git";
        reader->hash_algo = &hash_algos[GIT_HASH_SHA1];
+       strbuf_init(&reader->scratch, 0);
 }
 
 enum packet_read_status packet_reader_read(struct packet_reader *reader)
 {
-       struct strbuf scratch = STRBUF_INIT;
-
        if (reader->line_peeked) {
                reader->line_peeked = 0;
                return reader->status;
        }
 
+       if (reader->use_sideband)
+               reader->options |= PACKET_READ_USE_SIDEBAND;
+
        /*
         * Consume all progress packets until a primary payload packet is
         * received
@@ -621,7 +647,7 @@ enum packet_read_status packet_reader_read(struct packet_reader *reader)
                        break;
                if (demultiplex_sideband(reader->me, reader->status,
                                         reader->buffer, reader->pktlen, 1,
-                                        &scratch, &sideband_type))
+                                        &reader->scratch, &sideband_type))
                        break;
        }
 

diff --git a/pkt-line.h b/pkt-line.h
index aedef56286f4dde8968af3e4a9a33f05609f4e5c..3b33cc64f34dcc3447f6a6f42f12451dbc1a77a9 100644 (file)
--- a/pkt-line.h
+++ b/pkt-line.h
@@ -84,6 +84,7 @@ void packet_fflush(FILE *f);
 #define PACKET_READ_DIE_ON_ERR_PACKET    (1u<<2)
 #define PACKET_READ_GENTLE_ON_READ_ERROR (1u<<3)
 #define PACKET_READ_REDACT_URI_PATH      (1u<<4)
+#define PACKET_READ_USE_SIDEBAND         (1u<<5)
 int packet_read(int fd, char *buffer, unsigned size, int options);
 
 /*
@@ -193,6 +194,9 @@ struct packet_reader {
 
        /* hash algorithm in use */
        const struct git_hash_algo *hash_algo;
+
+       /* hold temporary sideband message */
+       struct strbuf scratch;
 };
 
 /*

diff --git a/t/helper/test-pkt-line.c b/t/helper/test-pkt-line.c
index 77e99c37df0cc00579083b53760accbd0888ffd1..4daa82f00fcb5f0b178aa912033c906fe14c7718 100644 (file)
--- a/t/helper/test-pkt-line.c
+++ b/t/helper/test-pkt-line.c
@@ -3,6 +3,7 @@
 #include "pkt-line.h"
 #include "sideband.h"
 #include "write-or-die.h"
+#include "parse-options.h"
 
 static void pack_line(const char *line)
 {
@@ -65,12 +66,33 @@ static void unpack(void)
        }
 }
 
-static void unpack_sideband(void)
+static void unpack_sideband(int argc, const char **argv)
 {
        struct packet_reader reader;
-       packet_reader_init(&reader, 0, NULL, 0,
-                          PACKET_READ_GENTLE_ON_EOF |
-                          PACKET_READ_CHOMP_NEWLINE);
+       int options = PACKET_READ_GENTLE_ON_EOF;
+       int chomp_newline = 1;
+       int reader_use_sideband = 0;
+       const char *const unpack_sideband_usage[] = {
+               "test_tool unpack_sideband [options...]", NULL
+       };
+       struct option cmd_options[] = {
+               OPT_BOOL(0, "reader-use-sideband", &reader_use_sideband,
+                        "set use_sideband bit for packet reader (Default: off)"),
+               OPT_BOOL(0, "chomp-newline", &chomp_newline,
+                        "chomp newline in packet (Default: on)"),
+               OPT_END()
+       };
+
+       argc = parse_options(argc, argv, "", cmd_options, unpack_sideband_usage,
+                            0);
+       if (argc > 0)
+               usage_msg_opt(_("too many arguments"), unpack_sideband_usage,
+                             cmd_options);
+
+       if (chomp_newline)
+               options |= PACKET_READ_CHOMP_NEWLINE;
+       packet_reader_init(&reader, 0, NULL, 0, options);
+       reader.use_sideband = reader_use_sideband;
 
        while (packet_reader_read(&reader) != PACKET_READ_EOF) {
                int band;
@@ -80,6 +102,17 @@ static void unpack_sideband(void)
                case PACKET_READ_EOF:
                        break;
                case PACKET_READ_NORMAL:
+                       /*
+                        * When the "use_sideband" field of the reader is turned
+                        * on, sideband packets other than the payload have been
+                        * parsed and consumed in packet_reader_read(), and only
+                        * the payload arrives here.
+                        */
+                       if (reader.use_sideband) {
+                               write_or_die(1, reader.line, reader.pktlen - 1);
+                               break;
+                       }
+
                        band = reader.line[0] & 0xff;
                        if (band < 1 || band > 2)
                                continue; /* skip non-sideband packets */
@@ -98,15 +131,31 @@ static void unpack_sideband(void)
 
 static int send_split_sideband(void)
 {
+       const char *foo = "Foo.\n";
+       const char *bar = "Bar.\n";
        const char *part1 = "Hello,";
        const char *primary = "\001primary: regular output\n";
        const char *part2 = " world!\n";
 
+       /* Each sideband message has a trailing newline character. */
+       send_sideband(1, 2, foo, strlen(foo), LARGE_PACKET_MAX);
+       send_sideband(1, 2, bar, strlen(bar), LARGE_PACKET_MAX);
+
+       /*
+        * One sideband message is divided into part1 and part2
+        * by the primary message.
+        */
        send_sideband(1, 2, part1, strlen(part1), LARGE_PACKET_MAX);
        packet_write(1, primary, strlen(primary));
        send_sideband(1, 2, part2, strlen(part2), LARGE_PACKET_MAX);
        packet_response_end(1);
 
+       /*
+        * We use unpack_sideband() to consume packets. A flush packet
+        * is required to end parsing.
+        */
+       packet_flush(1);
+
        return 0;
 }
 
@@ -127,7 +176,7 @@ int cmd__pkt_line(int argc, const char **argv)
        else if (!strcmp(argv[1], "unpack"))
                unpack();
        else if (!strcmp(argv[1], "unpack-sideband"))
-               unpack_sideband();
+               unpack_sideband(argc - 1, argv + 1);
        else if (!strcmp(argv[1], "send-split-sideband"))
                send_split_sideband();
        else if (!strcmp(argv[1], "receive-sideband"))

diff --git a/t/t0070-fundamental.sh b/t/t0070-fundamental.sh
index 487bc8d9054c908f97bc376005c3fd1eedd107b9..f18f9284a5bf3961a3f5373f887b808e55cd9b75 100755 (executable)
--- a/t/t0070-fundamental.sh
+++ b/t/t0070-fundamental.sh
@@ -53,4 +53,62 @@ test_expect_success 'missing sideband designator is reported' '
        test_grep "missing sideband" err
 '
 
+test_expect_success 'unpack-sideband: --no-chomp-newline' '
+       test_when_finished "rm -f expect-out expect-err" &&
+       test-tool pkt-line send-split-sideband >split-sideband &&
+       test-tool pkt-line unpack-sideband \
+               --no-chomp-newline <split-sideband >out 2>err &&
+       cat >expect-out <<-EOF &&
+               primary: regular output
+       EOF
+       cat >expect-err <<-EOF &&
+               Foo.
+               Bar.
+               Hello, world!
+       EOF
+       test_cmp expect-out out &&
+       test_cmp expect-err err
+'
+
+test_expect_success 'unpack-sideband: --chomp-newline (default)' '
+       test_when_finished "rm -f expect-out expect-err" &&
+       test-tool pkt-line send-split-sideband >split-sideband &&
+       test-tool pkt-line unpack-sideband \
+               --chomp-newline <split-sideband >out 2>err &&
+       printf "primary: regular output" >expect-out &&
+       printf "Foo.Bar.Hello, world!" >expect-err &&
+       test_cmp expect-out out &&
+       test_cmp expect-err err
+'
+
+test_expect_success 'unpack-sideband: packet_reader_read() consumes sideband, no chomp payload' '
+       test_when_finished "rm -f expect-out expect-err" &&
+       test-tool pkt-line send-split-sideband >split-sideband &&
+       test-tool pkt-line unpack-sideband \
+               --reader-use-sideband \
+               --no-chomp-newline <split-sideband >out 2>err &&
+       cat >expect-out <<-EOF &&
+               primary: regular output
+       EOF
+       printf "remote: Foo.        \n"           >expect-err &&
+       printf "remote: Bar.        \n"          >>expect-err &&
+       printf "remote: Hello, world!        \n" >>expect-err &&
+       test_cmp expect-out out &&
+       test_cmp expect-err err
+'
+
+test_expect_success 'unpack-sideband: packet_reader_read() consumes sideband, chomp payload' '
+       test_when_finished "rm -f expect-out expect-err" &&
+       test-tool pkt-line send-split-sideband >split-sideband &&
+       test-tool pkt-line unpack-sideband \
+               --reader-use-sideband \
+               --chomp-newline <split-sideband >out 2>err &&
+       printf "primary: regular output" >expect-out &&
+       printf "remote: Foo.        \n"           >expect-err &&
+       printf "remote: Bar.        \n"          >>expect-err &&
+       printf "remote: Hello, world!        \n" >>expect-err &&
+       test_cmp expect-out out &&
+       test_cmp expect-err err
+'
+
 test_done