Jeff KingWhen we fetch a symbolic ref file from the remote, we get the whole string "ref: refs/heads/master\n", recognize it by skipping past the "ref: ", and store the rest. We should chomp the trailing newline.
This bug was introduced in ae021d8 (use skip_prefix to avoid magic numbers, 2014-06-18), which did not notice that the length computation fed to xmemdupz was quietly tweaked by 1 to account for this.
We can solve it by explicitly trimming the newline, which is more obvious. Note that we use strbuf_rtrim here, which will actually cut off any trailing whitespace, not just a single newline. This is a good thing, though, as it makes our parsing more liberal (and spaces are not valid in refnames anyway).
Signed-off-by: Jeff King <peff@peff.net> --- This is a regression in v2.1.0.
It was causing t5540 to fail, but I realized I have been building with NO_EXPAT for a while, so I didn't notice. Frankly, I'm kind of surprised and disturbed that nobody noticed it before now. More evidence that we can kill off dumb http-push? I would have thought somebody else would have noticed the test failure, though.
I am embarrassed to have introduced the bug during a refactoring patch. But in my defense, the original code was quite subtle and horrible, and I think the end result at least is much obvious (and is a good point in favor of skip_prefix's existence!). The original came from eecc836 (Another memory overrun in http-push.c, 2007-03-01). Looking at that patch, I can't understand how the code before it ever worked in the first place.
http-push.c | 3 +++ 1 file changed, 3 insertions(+)
Show changes to http-push.c +3 −0
diff --git a/http-push.c b/http-push.c index 26dfa67..184d24a 100644 --- a/http-push.c +++ b/http-push.c @@ -1578,6 +1578,9 @@ static void fetch_symref(const char *path, char **symref, unsigned char *sha1) if (buffer.len == 0) return; + /* Cut off trailing newline. */ + strbuf_rtrim(&buffer); + /* If it's a symref, set the refname; otherwise try for a sha1 */ if (skip_prefix(buffer.buf, "ref: ", &name)) { *symref = xmemdupz(name, buffer.len - (name - buffer.buf));
-- 2.2.1.425.g441bb3c
