Merge branch 'tb/partial-clone-filters-fix'

gitster · gitster · commit 21127fa9829d · 2020-12-17T15:06:40.000-08:00
Fix potential server side resource deallocation issues when
responding to a partial clone request.

* tb/partial-clone-filters-fix:
  upload-pack.c: don't free allowed_filters util pointers
  builtin/clone.c: don't ignore transport_fetch_refs() errors
diff --git a/builtin/clone.c b/builtin/clone.c
@@ -1293,8 +1293,11 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 				break;
 			}
 
-		if (!is_local && !complete_refs_before_fetch)
-			transport_fetch_refs(transport, mapped_refs);
+		if (!is_local && !complete_refs_before_fetch) {
+			err = transport_fetch_refs(transport, mapped_refs);
+			if (err)
+				goto cleanup;
+		}
 
 		remote_head = find_ref_by_name(refs, "HEAD");
 		remote_head_points_at =
@@ -1339,8 +1342,11 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 
 	if (is_local)
 		clone_local(path, git_dir);
-	else if (refs && complete_refs_before_fetch)
-		transport_fetch_refs(transport, mapped_refs);
+	else if (refs && complete_refs_before_fetch) {
+		err = transport_fetch_refs(transport, mapped_refs);
+		if (err)
+			goto cleanup;
+	}
 
 	update_remote_refs(refs, mapped_refs, remote_head_points_at,
 			   branch_top.buf, reflog_msg.buf, transport,
@@ -1367,6 +1373,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 	junk_mode = JUNK_LEAVE_REPO;
 	err = checkout(submodule_progress);
 
+cleanup:
 	free(remote_name);
 	strbuf_release(&reflog_msg);
 	strbuf_release(&branch_top);
diff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh
@@ -289,7 +289,15 @@ test_expect_success 'upload-pack limits tree depth filters' '
 	test_config -C srv.bare uploadpackfilter.tree.maxDepth 0 &&
 	test_must_fail ok=sigpipe git clone --no-checkout --filter=tree:1 \
 		"file://$(pwd)/srv.bare" pc3 2>err &&
-	test_i18ngrep "tree filter allows max depth 0, but got 1" err
+	test_i18ngrep "tree filter allows max depth 0, but got 1" err &&
+
+	git clone --no-checkout --filter=tree:0 "file://$(pwd)/srv.bare" pc4 &&
+
+	test_config -C srv.bare uploadpackfilter.tree.maxDepth 5 &&
+	git clone --no-checkout --filter=tree:5 "file://$(pwd)/srv.bare" pc5 &&
+	test_must_fail ok=sigpipe git clone --no-checkout --filter=tree:6 \
+		"file://$(pwd)/srv.bare" pc6 2>err &&
+	test_i18ngrep "tree filter allows max depth 5, but got 6" err
 '
 
 test_expect_success 'partial clone fetches blobs pointed to by refs even if normally filtered out' '
diff --git a/upload-pack.c b/upload-pack.c
@@ -156,7 +156,7 @@ static void upload_pack_data_clear(struct upload_pack_data *data)
 	string_list_clear(&data->deepen_not, 0);
 	object_array_clear(&data->extra_edge_obj);
 	list_objects_filter_release(&data->filter_options);
-	string_list_clear(&data->allowed_filters, 1);
+	string_list_clear(&data->allowed_filters, 0);
 
 	free((char *)data->pack_objects_hook);
 }

Original file line number	Diff line number	Diff line change
`@@ -156,7 +156,7 @@ static void upload_pack_data_clear(struct upload_pack_data *data)`
`156`	`156`	`string_list_clear(&data->deepen_not, 0);`
`157`	`157`	`object_array_clear(&data->extra_edge_obj);`
`158`	`158`	`list_objects_filter_release(&data->filter_options);`
`159`		`- string_list_clear(&data->allowed_filters, 1);`
	`159`	`+ string_list_clear(&data->allowed_filters, 0);`
`160`	`160`
`161`	`161`	`free((char *)data->pack_objects_hook);`
`162`	`162`	`}`