Re: [PATCH v4] pkt-line: allow writing of LARGE_PACKET_MAX buffers
On Wed, Dec 10, 2014 at 4:47 AM, Jeff King <peff@peff.net> wrote:
Show 45 quoted lines
> Subject: pkt-line: allow writing of LARGE_PACKET_MAX buffers
>
> When we send out pkt-lines with refnames, we use a static
> 1000-byte buffer. This means that the maximum size of a ref
> over the git protocol is around 950 bytes (the exact size
> depends on the protocol line being written, but figure on a sha1
> plus some boilerplate).
>
> This is enough for any sane workflow, but occasionally odd
> things happen (e.g., a bug may create a ref "foo/foo/foo/..."
> accidentally). With the current code, you cannot even use
> "push" to delete such a ref from a remote.
>
> Let's switch to using a strbuf, with a hard-limit of
> LARGE_PACKET_MAX (which is specified by the protocol). This
> matches the size of the readers, as of 74543a0 (pkt-line:
> provide a LARGE_PACKET_MAX static buffer, 2013-02-20).
> Versions of git older than that will complain about our
> large packets, but it's really no worse than the current
> behavior. Right now the sender barfs with "impossibly long
> line" trying to send the packet, and afterwards the reader
> will barf with "protocol error: bad line length %d", which
> is arguably better anyway.
>
> Note that we're not really _solving_ the problem here, but
> just bumping the limits. In theory, the length of a ref is
> unbounded, and pkt-line can only represent sizes up to
> 65531 bytes. So we are just bumping the limit, not removing
> it. But hopefully 64K should be enough for anyone.
>
> As a bonus, by using a strbuf for the formatting we can
> eliminate an unnecessary copy in format_buf_write.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> diff --git a/t/t5527-fetch-odd-refs.sh b/t/t5527-fetch-odd-refs.sh
> index edea9f9..85bcb2e 100755
> --- a/t/t5527-fetch-odd-refs.sh
> +++ b/t/t5527-fetch-odd-refs.sh
> @@ -26,4 +26,37 @@ test_expect_success 'suffix ref is ignored during fetch' '
> test_cmp expect actual
> '
>
> +test_expect_success 'try to create repo with absurdly long refname' '
> + ref240=$_z40/$_z40/$_z40/$_z40/$_z40/$_z40
Maybe you want to keep the &&-chain intact here?
Show 35 quoted lines
> + ref1440=$ref240/$ref240/$ref240/$ref240/$ref240/$ref240 &&
> + git init long &&
> + (
> + cd long &&
> + test_commit long &&
> + test_commit master
> + ) &&
> + if git -C long update-ref refs/heads/$ref1440 long; then
> + test_set_prereq LONG_REF
> + else
> + echo >&2 "long refs not supported"
> + fi
> +'
> +
> +test_expect_success LONG_REF 'fetch handles extremely long refname' '
> + git fetch long refs/heads/*:refs/remotes/long/* &&
> + cat >expect <<-\EOF &&
> + long
> + master
> + EOF
> + git for-each-ref --format="%(subject)" refs/remotes/long >actual &&
> + test_cmp expect actual
> +'
> +
> +test_expect_success LONG_REF 'push handles extremely long refname' '
> + git push long :refs/heads/$ref1440 &&
> + git -C long for-each-ref --format="%(subject)" refs/heads >actual &&
> + echo master >expect &&
> + test_cmp expect actual
> +'
> +
> test_done
> --
> 2.2.0.454.g7eca6b7
>
Eric Sunshine <sunshine@sunshineco.com>