connect: also update offset for features without values

parse_feature_value() takes an offset, and uses it to seek past the point in features_list that we've already seen. However if the feature being searched for does not specify a value, the offset is not updated. Therefore if we call parse_feature_value() in a loop on a value-less feature, we'll keep on parsing the same feature over and over again. This usually isn't an issue: there's no point in using next_server_feature_value() to search for repeated instances of the same capability unless that capability typically specifies a value - but a broken server could send a response that omits the value for a feature even when we are expecting a value. Therefore we add an offset update calculation for the no-value case, which helps ensure that loops using next_server_feature_value() will always terminate. next_server_feature_value(), and the offset calculation, were first added in 2.28 in 2c6a403d96 (connect: add function to parse multiple v1 capability values, 2020-05-25). Thanks to Peff for authoring the test. Co-authored-by: Jeff King <peff@peff.net> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Andrzej Hunt <andrzej@ahunt.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Andrzej Hunt committed Sep 26, 2021 at 15:58 UTC 44d2aec6e8795ac22b9121b3d2ecf43589e8ecd2
2 files changed +17
connect.c
+2
@@ -555,6 +555,8 @@ const char *parse_feature_value(const char *feature_list, const char *feature, i
555 if (!*value || isspace(*value)) {
556 if (lenp)
557 *lenp = 0;
558 + if (offset)
559 + *offset = found + len - feature_list;
560 return value;
561 }
562 /* feature with a value (e.g., "agent=git/1.2.3") */
t/t5704-protocol-violations.sh
+15
@@ -32,4 +32,19 @@ test_expect_success 'extra delim packet in v2 fetch args' '
32 test_i18ngrep "expected flush after fetch arguments" err
33 '
34
35 +test_expect_success 'bogus symref in v0 capabilities' '
36 + test_commit foo &&
37 + oid=$(git rev-parse HEAD) &&
38 + dst=refs/heads/foo &&
39 + {
40 + printf "%s HEAD\0symref object-format=%s symref=HEAD:%s\n" \
41 + "$oid" "$GIT_DEFAULT_HASH" "$dst" |
42 + test-tool pkt-line pack-raw-stdin &&
43 + printf "0000"
44 + } >input &&
45 + git ls-remote --symref --upload-pack="cat input; read junk;:" . >actual &&
46 + printf "ref: %s\tHEAD\n%s\tHEAD\n" "$dst" "$oid" >expect &&
47 + test_cmp expect actual
48 +'
49 +
50 test_done