Re: problem with http clone/pull
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:42:40
Subsystem:
the rest · Maintainer:
Linus Torvalds
Possibly related (same subject, not in this thread)
- 2016-06-15 · problem with http clone/pull · Paul Mackerras <hidden>
- 2016-06-15 · Re: problem with http clone/pull · Junio C Hamano <hidden>
- 2016-06-15 · Re: problem with http clone/pull · Junio C Hamano <hidden>
- 2016-06-15 · Re: problem with http clone/pull · Paul Mackerras <hidden>
- 2016-06-15 · Re: problem with http clone/pull · Junio C Hamano <hidden>
Junio C Hamano [off-list ref] writes:
Perhaps this would fix it? I am at work now and I haven't looked at the logic aruond it too deeply (e.g. I do not know if this breaks the relative alternate or http specific cases, nor the same or similar breakages were there in these other cases in the original code to begin with)
Side note:
quoted hunk
diff --git a/http-fetch.c b/http-fetch.c index fac1760..d870390 100644 --- a/http-fetch.c +++ b/http-fetch.c@@ -559,7 +559,13 @@ static void process_alternates_response( char *target = NULL; char *path; if (data[i] == '/') { - serverlen = strchr(base + 8, '/') - base; + /* This counts + * http://git.host/pub/scm/linux.git + * 1234567----here^ + * so strcpy(dst, base, serverlen) will + * copy up to "...git.host/" + */ + serverlen = strchr(base + 7, '/') - base; okay = 1; } else if (!memcmp(data + i, "../", 3)) { i += 3;
The change between 7 and 8 does not really matter, because the hostname cannot be empty, and 8 was masking the breakage of this code; it was (perhaps deliberately) being sloppy to allow us to also skip over "protocol://" part for https:// case. Call it subtle if you want ;-). I think the right thing for this part to do would be something like this:
diff --git a/http-fetch.c b/http-fetch.c
index fac1760..c7545f2 100644
--- a/http-fetch.c
+++ b/http-fetch.c@@ -559,8 +559,18 @@ static void process_alternates_response( char *target = NULL; char *path; if (data[i] == '/') { - serverlen = strchr(base + 8, '/') - base; - okay = 1; + /* This counts + * http://git.host/pub/scm/linux.git + * -----------here^ + * so memcpy(dst, base, serverlen) will + * copy up to "...git.host". + */ + const char *colon_ss = strstr(base,"://"); + if (colon_ss) { + serverlen = (strchr(colon_ss + 3, '/') + - base); + okay = 1; + } } else if (!memcmp(data + i, "../", 3)) { i += 3; serverlen = strlen(base);
@@ -583,11 +593,13 @@ static void process_alternates_response( okay = 1; } } - /* skip 'objects' at end */ + /* skip "objects\n" at end */ if (okay) { target = xmalloc(serverlen + posn - i - 6); - strlcpy(target, base, serverlen); - strlcpy(target + serverlen, data + i, posn - i - 6); + memcpy(target, base, serverlen); + memcpy(target + serverlen, data + i, + posn - i - 7); + target[serverlen + posn - i - 7] = 0; if (get_verbosely) fprintf(stderr, "Also look at %s\n", target);