Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Merge branch 'jk/check-corrupt-objects-carefully'

Have the streaming interface and other codepaths more carefully
examine for corrupt objects.

* jk/check-corrupt-objects-carefully:
  clone: leave repo in place after checkout errors
  clone: run check_everything_connected
  clone: die on errors from unpack_trees
  add tests for cloning corrupted repositories
  streaming_write_entry: propagate streaming errors
  add test for streaming corrupt blobs
  avoid infinite loop in read_istream_loose
  read_istream_filtered: propagate read error from upstream
  check_sha1_signature: check return value from read_istream
  stream_blob_to_fd: detect errors reading from stream
  • Loading branch information...
commit b9c78e97237df7df45549d29755e51b4a0fdc5ea 2 parents a70f4cb + d3b3462
@gitster gitster authored
View
54 builtin/clone.c
@@ -23,6 +23,7 @@
#include "branch.h"
#include "remote.h"
#include "run-command.h"
+#include "connected.h"
/*
* Overall FIXMEs:
@@ -376,10 +377,32 @@ static void clone_local(const char *src_repo, const char *dest_repo)
static const char *junk_work_tree;
static const char *junk_git_dir;
static pid_t junk_pid;
+enum {
+ JUNK_LEAVE_NONE,
+ JUNK_LEAVE_REPO,
+ JUNK_LEAVE_ALL
+} junk_mode = JUNK_LEAVE_NONE;
+
+static const char junk_leave_repo_msg[] =
+N_("Clone succeeded, but checkout failed.\n"
+ "You can inspect what was checked out with 'git status'\n"
+ "and retry the checkout with 'git checkout -f HEAD'\n");
static void remove_junk(void)
{
struct strbuf sb = STRBUF_INIT;
+
+ switch (junk_mode) {
+ case JUNK_LEAVE_REPO:
+ warning("%s", _(junk_leave_repo_msg));
+ /* fall-through */
+ case JUNK_LEAVE_ALL:
+ return;
+ default:
+ /* proceed to removal */
+ break;
+ }
+
if (getpid() != junk_pid)
return;
if (junk_git_dir) {
@@ -485,12 +508,37 @@ static void write_followtags(const struct ref *refs, const char *msg)
}
}
+static int iterate_ref_map(void *cb_data, unsigned char sha1[20])
+{
+ struct ref **rm = cb_data;
+ struct ref *ref = *rm;
+
+ /*
+ * Skip anything missing a peer_ref, which we are not
+ * actually going to write a ref for.
+ */
+ while (ref && !ref->peer_ref)
+ ref = ref->next;
+ /* Returning -1 notes "end of list" to the caller. */
+ if (!ref)
+ return -1;
+
+ hashcpy(sha1, ref->old_sha1);
+ *rm = ref->next;
+ return 0;
+}
+
static void update_remote_refs(const struct ref *refs,
const struct ref *mapped_refs,
const struct ref *remote_head_points_at,
const char *branch_top,
const char *msg)
{
+ const struct ref *rm = mapped_refs;
+
+ if (check_everything_connected(iterate_ref_map, 0, &rm))
+ die(_("remote did not send all necessary objects"));
+
if (refs) {
write_remote_refs(mapped_refs);
if (option_single_branch)
@@ -579,7 +627,8 @@ static int checkout(void)
tree = parse_tree_indirect(sha1);
parse_tree(tree);
init_tree_desc(&t, tree->buffer, tree->size);
- unpack_trees(1, &t, &opts);
+ if (unpack_trees(1, &t, &opts) < 0)
+ die(_("unable to checkout working tree"));
if (write_cache(fd, active_cache, active_nr) ||
commit_locked_index(lock_file))
@@ -898,12 +947,13 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
transport_unlock_pack(transport);
transport_disconnect(transport);
+ junk_mode = JUNK_LEAVE_REPO;
err = checkout();
strbuf_release(&reflog_msg);
strbuf_release(&branch_top);
strbuf_release(&key);
strbuf_release(&value);
- junk_pid = 0;
+ junk_mode = JUNK_LEAVE_ALL;
return err;
}
View
16 entry.c
@@ -120,16 +120,18 @@ static int streaming_write_entry(struct cache_entry *ce, char *path,
const struct checkout *state, int to_tempfile,
int *fstat_done, struct stat *statbuf)
{
- int result = -1;
+ int result = 0;
int fd;
fd = open_output_fd(path, ce, to_tempfile);
- if (0 <= fd) {
- result = stream_blob_to_fd(fd, ce->sha1, filter, 1);
- *fstat_done = fstat_output(fd, state, statbuf);
- result = close(fd);
- }
- if (result && 0 <= fd)
+ if (fd < 0)
+ return -1;
+
+ result |= stream_blob_to_fd(fd, ce->sha1, filter, 1);
+ *fstat_done = fstat_output(fd, state, statbuf);
+ result |= close(fd);
+
+ if (result)
unlink(path);
return result;
}
View
4 sha1_file.c
@@ -1271,6 +1271,10 @@ int check_sha1_signature(const unsigned char *sha1, void *map,
char buf[1024 * 16];
ssize_t readlen = read_istream(st, buf, sizeof(buf));
+ if (readlen < 0) {
+ close_istream(st);
+ return -1;
+ }
if (!readlen)
break;
git_SHA1_Update(&c, buf, readlen);
View
6 streaming.c
@@ -237,7 +237,7 @@ static read_method_decl(filtered)
if (!fs->input_finished) {
fs->i_end = read_istream(fs->upstream, fs->ibuf, FILTER_BUFFER);
if (fs->i_end < 0)
- break;
+ return -1;
if (fs->i_end)
continue;
}
@@ -309,7 +309,7 @@ static read_method_decl(loose)
st->z_state = z_done;
break;
}
- if (status != Z_OK && status != Z_BUF_ERROR) {
+ if (status != Z_OK && (status != Z_BUF_ERROR || total_read < sz)) {
git_inflate_end(&st->z);
st->z_state = z_error;
return -1;
@@ -514,6 +514,8 @@ int stream_blob_to_fd(int fd, unsigned const char *sha1, struct stream_filter *f
ssize_t wrote, holeto;
ssize_t readlen = read_istream(st, buf, sizeof(buf));
+ if (readlen < 0)
+ goto close_and_exit;
if (!readlen)
break;
if (can_seek && sizeof(buf) == readlen) {
View
104 t/t1060-object-corruption.sh
@@ -0,0 +1,104 @@
+#!/bin/sh
+
+test_description='see how we handle various forms of corruption'
+. ./test-lib.sh
+
+# convert "1234abcd" to ".git/objects/12/34abcd"
+obj_to_file() {
+ echo "$(git rev-parse --git-dir)/objects/$(git rev-parse "$1" | sed 's,..,&/,')"
+}
+
+# Convert byte at offset "$2" of object "$1" into '\0'
+corrupt_byte() {
+ obj_file=$(obj_to_file "$1") &&
+ chmod +w "$obj_file" &&
+ printf '\0' | dd of="$obj_file" bs=1 seek="$2" conv=notrunc
+}
+
+test_expect_success 'setup corrupt repo' '
+ git init bit-error &&
+ (
+ cd bit-error &&
+ test_commit content &&
+ corrupt_byte HEAD:content.t 10
+ )
+'
+
+test_expect_success 'setup repo with missing object' '
+ git init missing &&
+ (
+ cd missing &&
+ test_commit content &&
+ rm -f "$(obj_to_file HEAD:content.t)"
+ )
+'
+
+test_expect_success 'setup repo with misnamed object' '
+ git init misnamed &&
+ (
+ cd misnamed &&
+ test_commit content &&
+ good=$(obj_to_file HEAD:content.t) &&
+ blob=$(echo corrupt | git hash-object -w --stdin) &&
+ bad=$(obj_to_file $blob) &&
+ rm -f "$good" &&
+ mv "$bad" "$good"
+ )
+'
+
+test_expect_success 'streaming a corrupt blob fails' '
+ (
+ cd bit-error &&
+ test_must_fail git cat-file blob HEAD:content.t
+ )
+'
+
+test_expect_success 'read-tree -u detects bit-errors in blobs' '
+ (
+ cd bit-error &&
+ rm -f content.t &&
+ test_must_fail git read-tree --reset -u HEAD
+ )
+'
+
+test_expect_success 'read-tree -u detects missing objects' '
+ (
+ cd missing &&
+ rm -f content.t &&
+ test_must_fail git read-tree --reset -u HEAD
+ )
+'
+
+# We use --bare to make sure that the transport detects it, not the checkout
+# phase.
+test_expect_success 'clone --no-local --bare detects corruption' '
+ test_must_fail git clone --no-local --bare bit-error corrupt-transport
+'
+
+test_expect_success 'clone --no-local --bare detects missing object' '
+ test_must_fail git clone --no-local --bare missing missing-transport
+'
+
+test_expect_success 'clone --no-local --bare detects misnamed object' '
+ test_must_fail git clone --no-local --bare misnamed misnamed-transport
+'
+
+# We do not expect --local to detect corruption at the transport layer,
+# so we are really checking the checkout() code path.
+test_expect_success 'clone --local detects corruption' '
+ test_must_fail git clone --local bit-error corrupt-checkout
+'
+
+test_expect_success 'error detected during checkout leaves repo intact' '
+ test_path_is_dir corrupt-checkout/.git
+'
+
+test_expect_success 'clone --local detects missing objects' '
+ test_must_fail git clone --local missing missing-checkout
+'
+
+test_expect_failure 'clone --local detects misnamed objects' '
+ test_must_fail git clone --local misnamed misnamed-checkout
+'
+
+test_done
View
8 t/t5710-info-alternate.sh
@@ -58,13 +58,7 @@ test_expect_success 'creating too deep nesting' \
git clone -l -s D E &&
git clone -l -s E F &&
git clone -l -s F G &&
-git clone -l -s G H'
-
-test_expect_success 'invalidity of deepest repository' \
-'cd H && {
- test_valid_repo
- test $? -ne 0
-}'
+test_must_fail git clone --bare -l -s G H'
cd "$base_dir"
Please sign in to comment.
Something went wrong with that request. Please try again.