http-fetch: Use temporary files for pack-*.idx until verified
Verify that a downloaded pack-*.idx file is consistent and valid as an index file before we rename it into its final destination. This prevents a corrupt index file from later being treated as a usable file, confusing readers. Check that we do not have the pack index file before invoking fetch_pack_index(); that way, we can do without the has_pack_index() check in fetch_pack_index(). Signed-off-by: Shawn O. Pearce <spearce@spearce.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Shawn O. Pearce committed
Apr 19, 2010 at 07:23 UTC
750ef42516bb343a7755f003720e43cd8dd64c3e
2 files changed
+54
-17
http.c
+39
-17
@@ -897,18 +897,11 @@ int http_fetch_ref(const char *base, struct ref *ref)
897
}
898
899
/* Helpers for fetching packs */
900
-static int fetch_pack_index(unsigned char *sha1, const char *base_url)
900
+static char *fetch_pack_index(unsigned char *sha1, const char *base_url)
901
{
902
- int ret = 0;
903
- char *filename;
904
- char *url = NULL;
902
+ char *url, *tmp;
903
struct strbuf buf = STRBUF_INIT;
904
907
- if (has_pack_index(sha1)) {
908
- ret = 0;
909
- goto cleanup;
910
- }
911
-
905
if (http_is_verbose)
906
fprintf(stderr, "Getting index for pack %s\n", sha1_to_hex(sha1));
907
@@ -916,26 +909,55 @@ static int fetch_pack_index(unsigned char *sha1, const char *base_url)
909
strbuf_addf(&buf, "objects/pack/pack-%s.idx", sha1_to_hex(sha1));
910
url = strbuf_detach(&buf, NULL);
911
919
- filename = sha1_pack_index_name(sha1);
920
- if (http_get_file(url, filename, 0) != HTTP_OK)
921
- ret = error("Unable to get pack index %s\n", url);
912
+ strbuf_addf(&buf, "%s.temp", sha1_pack_index_name(sha1));
913
+ tmp = strbuf_detach(&buf, NULL);
914
+
915
+ if (http_get_file(url, tmp, 0) != HTTP_OK) {
916
+ error("Unable to get pack index %s\n", url);
917
+ free(tmp);
918
+ tmp = NULL;
919
+ }
920
923
-cleanup:
921
free(url);
925
- return ret;
922
+ return tmp;
923
}
924
925
static int fetch_and_setup_pack_index(struct packed_git **packs_head,
926
unsigned char *sha1, const char *base_url)
927
{
928
struct packed_git *new_pack;
929
+ char *tmp_idx = NULL;
930
+ int ret;
931
+
932
+ if (has_pack_index(sha1)) {
933
+ new_pack = parse_pack_index(sha1, NULL);
934
+ if (!new_pack)
935
+ return -1; /* parse_pack_index() already issued error message */
936
+ goto add_pack;
937
+ }
938
933
- if (fetch_pack_index(sha1, base_url))
939
+ tmp_idx = fetch_pack_index(sha1, base_url);
940
+ if (!tmp_idx)
941
return -1;
942
936
- new_pack = parse_pack_index(sha1, sha1_pack_index_name(sha1));
937
- if (!new_pack)
943
+ new_pack = parse_pack_index(sha1, tmp_idx);
944
+ if (!new_pack) {
945
+ unlink(tmp_idx);
946
+ free(tmp_idx);
947
+
948
return -1; /* parse_pack_index() already issued error message */
949
+ }
950
+
951
+ ret = verify_pack_index(new_pack);
952
+ if (!ret) {
953
+ close_pack_index(new_pack);
954
+ ret = move_temp_to_file(tmp_idx, sha1_pack_index_name(sha1));
955
+ }
956
+ free(tmp_idx);
957
+ if (ret)
958
+ return -1;
959
+
960
+add_pack:
961
new_pack->next = *packs_head;
962
*packs_head = new_pack;
963
return 0;
t/t5550-http-fetch.sh
+15
@@ -77,6 +77,21 @@ test_expect_success 'fetch notices corrupt pack' '
77
)
78
'
79
80
+test_expect_success 'fetch notices corrupt idx' '
81
+ cp -R "$HTTPD_DOCUMENT_ROOT_PATH"/repo_pack.git "$HTTPD_DOCUMENT_ROOT_PATH"/repo_bad2.git &&
82
+ (cd "$HTTPD_DOCUMENT_ROOT_PATH"/repo_bad2.git &&
83
+ p=`ls objects/pack/pack-*.idx` &&
84
+ chmod u+w $p &&
85
+ printf %0256d 0 | dd of=$p bs=256 count=1 seek=1 conv=notrunc
86
+ ) &&
87
+ mkdir repo_bad2.git &&
88
+ (cd repo_bad2.git &&
89
+ git --bare init &&
90
+ test_must_fail git --bare fetch $HTTPD_URL/dumb/repo_bad2.git &&
91
+ test 0 = `ls objects/pack | wc -l`
92
+ )
93
+'
94
+
95
test_expect_success 'did not use upload-pack service' '
96
grep '/git-upload-pack' <"$HTTPD_ROOT_PATH"/access.log >act
97
: >exp