diff --git a/gvfs-helper.c b/gvfs-helper.c index 87f1a8ec563f55..3d6ca2b9c1708f 100644 --- a/gvfs-helper.c +++ b/gvfs-helper.c @@ -2121,7 +2121,6 @@ static void extract_packfile_from_multipack( { struct ph ph; struct tempfile *tempfile_pack = NULL; - struct tempfile *tempfile_idx = NULL; int result = -1; int b_no_idx_in_multipack; struct object_id packfile_checksum; @@ -2155,16 +2154,14 @@ static void extract_packfile_from_multipack( b_no_idx_in_multipack = (ph.idx_len == maximum_unsigned_value_of_type(uint64_t) || ph.idx_len == 0); - if (b_no_idx_in_multipack) { - my_create_tempfile(status, 0, "pack", &tempfile_pack, NULL, NULL); - if (!tempfile_pack) - goto done; - } else { - /* create a pair of tempfiles with the same basename */ - my_create_tempfile(status, 0, "pack", &tempfile_pack, "idx", &tempfile_idx); - if (!tempfile_pack || !tempfile_idx) - goto done; - } + /* + * We are going to harden `gvfs-helper` here and ignore the .idx file + * if it is provided and always compute it locally so that we get the + * added verification that `git index-pack` provides. + */ + my_create_tempfile(status, 0, "pack", &tempfile_pack, NULL, NULL); + if (!tempfile_pack) + goto done; /* * Copy the current packfile from the open stream and capture @@ -2191,38 +2188,31 @@ static void extract_packfile_from_multipack( oid_to_hex_r(hex_checksum, &packfile_checksum); - if (b_no_idx_in_multipack) { - /* - * The server did not send the corresponding .idx, so - * we have to compute it ourselves. - */ - strbuf_addbuf(&temp_path_idx, &temp_path_pack); - strbuf_strip_suffix(&temp_path_idx, ".pack"); - strbuf_addstr(&temp_path_idx, ".idx"); + /* + * Always compute the .idx file from the .pack file. + */ + strbuf_addbuf(&temp_path_idx, &temp_path_pack); + strbuf_strip_suffix(&temp_path_idx, ".pack"); + strbuf_addstr(&temp_path_idx, ".idx"); - my_run_index_pack(params, status, - &temp_path_pack, &temp_path_idx, - NULL); - if (status->ec != GH__ERROR_CODE__OK) - goto done; + my_run_index_pack(params, status, + &temp_path_pack, &temp_path_idx, + NULL); + if (status->ec != GH__ERROR_CODE__OK) + goto done; - } else { + if (!b_no_idx_in_multipack) { /* * Server sent the .idx immediately after the .pack in the - * data stream. I'm tempted to verify it, but that defeats - * the purpose of having it cached... + * data stream. Skip over it. */ - if (my_copy_fd_len(fd_multipack, get_tempfile_fd(tempfile_idx), - ph.idx_len) < 0) { + if (lseek(fd_multipack, ph.idx_len, SEEK_CUR) < 0) { strbuf_addf(&status->error_message, - "could not extract index[%d] in multipack", + "could not skip index[%d] in multipack", k); status->ec = GH__ERROR_CODE__COULD_NOT_INSTALL_PREFETCH; goto done; } - - strbuf_addstr(&temp_path_idx, get_tempfile_path(tempfile_idx)); - close_tempfile_gently(tempfile_idx); } strbuf_addf(&buf_timestamp, "%u", (unsigned int)ph.timestamp); @@ -2237,7 +2227,6 @@ static void extract_packfile_from_multipack( done: delete_tempfile(&tempfile_pack); - delete_tempfile(&tempfile_idx); strbuf_release(&temp_path_pack); strbuf_release(&temp_path_idx); strbuf_release(&final_path_pack); diff --git a/t/t5799-gvfs-helper.sh b/t/t5799-gvfs-helper.sh index 8ceb71e69b4205..73604aabb78f7f 100755 --- a/t/t5799-gvfs-helper.sh +++ b/t/t5799-gvfs-helper.sh @@ -1367,14 +1367,11 @@ test_expect_success 'prefetch corrupt pack without idx' ' # Send corrupt PACK files with IDX files. Since the cache server # sends both, `gvfs-helper` might fail to verify both of them. -test_expect_failure 'prefetch corrupt pack with corrupt idx' ' +test_expect_success 'prefetch corrupt pack with corrupt idx' ' test_when_finished "per_test_cleanup" && start_gvfs_protocol_server_with_mayhem \ bad_prefetch_pack_sha && - # TODO This is a false-positive since `gvfs-helper` - # TODO does not verify either of them when a pair - # TODO is sent. test_must_fail \ git -C "$REPO_T1" gvfs-helper \ --cache-server=disable \