http-push: refactor parsing of remote object names

We get loose object names like "objects/??/..." from the remote side, and need to convert them to their hex representation. The code to do so is rather hard to follow, as it uses some calculated lengths whose origins are hard to understand and verify (e.g., the path must be exactly 49 characters long. why? Why doesn't the strcpy overflow obj_hex, which is the same length as path?). We can simplify this a bit by using skip_prefix, using standard 40- and 20-character buffers for hex and binary sha1s, and adding some comments. We also drop a totally bogus comment that claims strlcpy cannot be used because "path" is not NUL-terminated. Right between a call to strlen(path) and strcpy(path). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Jun 19, 2014 at 17:58 UTC 67a31f612830e79fe768ac886ed9ef7eadd8fb10
1 file changed +23 -15
http-push.c
+23 -15
@@ -719,14 +719,10 @@ static int fetch_indices(void)
719 return ret;
720 }
721
722 -static void one_remote_object(const char *hex)
722 +static void one_remote_object(const unsigned char *sha1)
723 {
724 - unsigned char sha1[20];
724 struct object *obj;
725
727 - if (get_sha1_hex(hex, sha1) != 0)
728 - return;
729 -
726 obj = lookup_object(sha1);
727 if (!obj)
728 obj = parse_object(sha1);
@@ -1020,26 +1016,38 @@ static void remote_ls(const char *path, int flags,
1016 void (*userFunc)(struct remote_ls_ctx *ls),
1017 void *userData);
1018
1019 +/* extract hex from sharded "xx/x{40}" filename */
1020 +static int get_sha1_hex_from_objpath(const char *path, unsigned char *sha1)
1021 +{
1022 + char hex[40];
1023 +
1024 + if (strlen(path) != 41)
1025 + return -1;
1026 +
1027 + memcpy(hex, path, 2);
1028 + path += 2;
1029 + path++; /* skip '/' */
1030 + memcpy(hex, path, 38);
1031 +
1032 + return get_sha1_hex(hex, sha1);
1033 +}
1034 +
1035 static void process_ls_object(struct remote_ls_ctx *ls)
1036 {
1037 unsigned int *parent = (unsigned int *)ls->userData;
1026 - char *path = ls->dentry_name;
1027 - char *obj_hex;
1038 + const char *path = ls->dentry_name;
1039 + unsigned char sha1[20];
1040
1041 if (!strcmp(ls->path, ls->dentry_name) && (ls->flags & IS_DIR)) {
1042 remote_dir_exists[*parent] = 1;
1043 return;
1044 }
1045
1034 - if (strlen(path) != 49)
1046 + if (!skip_prefix(path, "objects/", &path) ||
1047 + get_sha1_hex_from_objpath(path, sha1))
1048 return;
1036 - path += 8;
1037 - obj_hex = xmalloc(strlen(path));
1038 - /* NB: path is not null-terminated, can not use strlcpy here */
1039 - memcpy(obj_hex, path, 2);
1040 - strcpy(obj_hex + 2, path + 3);
1041 - one_remote_object(obj_hex);
1042 - free(obj_hex);
1049 +
1050 + one_remote_object(sha1);
1051 }
1052
1053 static void process_ls_ref(struct remote_ls_ctx *ls)