Commit 619f61a8f110de71422d62e14e22c84865c3091c

Edward Thomson 2018-02-01T06:22:36

odb: error when we can't create object header Return an error to the caller when we can't create an object header for some reason (printf failure) instead of simply asserting.

diff --git a/src/indexer.c b/src/indexer.c
index a5e8422..c0976f2 100644
--- a/src/indexer.c
+++ b/src/indexer.c
@@ -187,13 +187,17 @@ static int store_delta(git_indexer *idx)
 	return 0;
 }
 
-static void hash_header(git_hash_ctx *ctx, git_off_t len, git_otype type)
+static int hash_header(git_hash_ctx *ctx, git_off_t len, git_otype type)
 {
 	char buffer[64];
 	size_t hdrlen;
+	int error;
+
+	if ((error = git_odb__format_object_header(&hdrlen,
+		buffer, sizeof(buffer), (size_t)len, type)) < 0)
+		return error;
 
-	hdrlen = git_odb__format_object_header(buffer, sizeof(buffer), (size_t)len, type);
-	git_hash_update(ctx, buffer, hdrlen);
+	return git_hash_update(ctx, buffer, hdrlen);
 }
 
 static int hash_object_stream(git_indexer*idx, git_packfile_stream *stream)
@@ -621,7 +625,10 @@ int git_indexer_append(git_indexer *idx, const void *data, size_t size, git_tran
 				idx->have_delta = 1;
 			} else {
 				idx->have_delta = 0;
-				hash_header(&idx->hash_ctx, entry_size, type);
+
+				error = hash_header(&idx->hash_ctx, entry_size, type);
+				if (error < 0)
+					goto on_error;
 			}
 
 			idx->have_stream = 1;
diff --git a/src/odb.c b/src/odb.c
index 57f01ee..802c721 100644
--- a/src/odb.c
+++ b/src/odb.c
@@ -84,19 +84,34 @@ static int odb_read_hardcoded(bool *found, git_rawobj *raw, const git_oid *id)
 	return 0;
 }
 
-int git_odb__format_object_header(char *hdr, size_t n, git_off_t obj_len, git_otype obj_type)
+int git_odb__format_object_header(
+	size_t *written,
+	char *hdr,
+	size_t hdr_size,
+	git_off_t obj_len,
+	git_otype obj_type)
 {
 	const char *type_str = git_object_type2string(obj_type);
-	int len = p_snprintf(hdr, n, "%s %lld", type_str, (long long)obj_len);
-	assert(len > 0 && len <= (int)n);
-	return len+1;
+	int hdr_max = (hdr_size > INT_MAX-2) ? (INT_MAX-2) : (int)hdr_size;
+	int len;
+
+	len = p_snprintf(hdr, hdr_max, "%s %lld", type_str, (long long)obj_len);
+
+	if (len < 0 || len >= hdr_max) {
+		giterr_set(GITERR_OS, "object header creation failed");
+		return -1;
+	}
+
+	*written = (size_t)(len + 1);
+	return 0;
 }
 
 int git_odb__hashobj(git_oid *id, git_rawobj *obj)
 {
 	git_buf_vec vec[2];
 	char header[64];
-	int hdrlen;
+	size_t hdrlen;
+	int error;
 
 	assert(id && obj);
 
@@ -110,16 +125,16 @@ int git_odb__hashobj(git_oid *id, git_rawobj *obj)
 		return -1;
 	}
 
-	hdrlen = git_odb__format_object_header(header, sizeof(header), obj->len, obj->type);
+	if ((error = git_odb__format_object_header(&hdrlen,
+		header, sizeof(header), obj->len, obj->type)) < 0)
+		return error;
 
 	vec[0].data = header;
 	vec[0].len = hdrlen;
 	vec[1].data = obj->data;
 	vec[1].len = obj->len;
 
-	git_hash_vec(id, vec, 2);
-
-	return 0;
+	return git_hash_vec(id, vec, 2);
 }
 
 
@@ -182,7 +197,7 @@ void git_odb_object_free(git_odb_object *object)
 
 int git_odb__hashfd(git_oid *out, git_file fd, size_t size, git_otype type)
 {
-	int hdr_len;
+	size_t hdr_len;
 	char hdr[64], buffer[FILEIO_BUFSIZE];
 	git_hash_ctx ctx;
 	ssize_t read_len = 0;
@@ -196,7 +211,9 @@ int git_odb__hashfd(git_oid *out, git_file fd, size_t size, git_otype type)
 	if ((error = git_hash_ctx_init(&ctx)) < 0)
 		return error;
 
-	hdr_len = git_odb__format_object_header(hdr, sizeof(hdr), size, type);
+	if ((error = git_odb__format_object_header(&hdr_len, hdr,
+		sizeof(hdr), size, type)) < 0)
+		goto done;
 
 	if ((error = git_hash_update(&ctx, hdr, hdr_len)) < 0)
 		goto done;
@@ -1296,13 +1313,17 @@ int git_odb_write(
 	return error;
 }
 
-static void hash_header(git_hash_ctx *ctx, git_off_t size, git_otype type)
+static int hash_header(git_hash_ctx *ctx, git_off_t size, git_otype type)
 {
 	char header[64];
-	int hdrlen;
+	size_t hdrlen;
+	int error;
 
-	hdrlen = git_odb__format_object_header(header, sizeof(header), size, type);
-	git_hash_update(ctx, header, hdrlen);
+	 if ((error = git_odb__format_object_header(&hdrlen,
+		header, sizeof(header), size, type)) < 0)
+		return error;
+
+	return git_hash_update(ctx, header, hdrlen);
 }
 
 int git_odb_open_wstream(
@@ -1343,12 +1364,11 @@ int git_odb_open_wstream(
 	ctx = git__malloc(sizeof(git_hash_ctx));
 	GITERR_CHECK_ALLOC(ctx);
 
-	if ((error = git_hash_ctx_init(ctx)) < 0)
+	if ((error = git_hash_ctx_init(ctx)) < 0 ||
+		(error = hash_header(ctx, size, type)) < 0)
 		goto done;
 
-	hash_header(ctx, size, type);
 	(*stream)->hash_ctx = ctx;
-
 	(*stream)->declared_size = size;
 	(*stream)->received_bytes = 0;
 
diff --git a/src/odb.h b/src/odb.h
index 6845b22..b354108 100644
--- a/src/odb.h
+++ b/src/odb.h
@@ -70,7 +70,7 @@ int git_odb__hashobj(git_oid *id, git_rawobj *obj);
 /*
  * Format the object header such as it would appear in the on-disk object
  */
-int git_odb__format_object_header(char *hdr, size_t n, git_off_t obj_len, git_otype obj_type);
+int git_odb__format_object_header(size_t *out_len, char *hdr, size_t hdr_size, git_off_t obj_len, git_otype obj_type);
 /*
  * Hash an open file descriptor.
  * This is a performance call when the contents of a fd need to be hashed,
diff --git a/src/odb_loose.c b/src/odb_loose.c
index 7d77eed..713288d 100644
--- a/src/odb_loose.c
+++ b/src/odb_loose.c
@@ -824,14 +824,17 @@ static int loose_backend__writestream(git_odb_stream **stream_out, git_odb_backe
 	loose_writestream *stream = NULL;
 	char hdr[MAX_HEADER_LEN];
 	git_buf tmp_path = GIT_BUF_INIT;
-	int hdrlen;
+	size_t hdrlen;
+	int error;
 
 	assert(_backend && length >= 0);
 
 	backend = (loose_backend *)_backend;
 	*stream_out = NULL;
 
-	hdrlen = git_odb__format_object_header(hdr, sizeof(hdr), length, type);
+	if ((error = git_odb__format_object_header(&hdrlen,
+		hdr, sizeof(hdr), length, type)) < 0)
+		return error;
 
 	stream = git__calloc(1, sizeof(loose_writestream));
 	GITERR_CHECK_ALLOC(stream);
@@ -1036,16 +1039,19 @@ done:
 
 static int loose_backend__write(git_odb_backend *_backend, const git_oid *oid, const void *data, size_t len, git_otype type)
 {
-	int error = 0, header_len;
+	int error = 0;
 	git_buf final_path = GIT_BUF_INIT;
 	char header[MAX_HEADER_LEN];
+	size_t header_len;
 	git_filebuf fbuf = GIT_FILEBUF_INIT;
 	loose_backend *backend;
 
 	backend = (loose_backend *)_backend;
 
 	/* prepare the header for the file */
-	header_len = git_odb__format_object_header(header, sizeof(header), len, type);
+	if ((error = git_odb__format_object_header(&header_len,
+		header, sizeof(header), len, type)) < 0)
+		goto cleanup;
 
 	if (git_buf_joinpath(&final_path, backend->objects_dir, "tmp_object") < 0 ||
 		git_filebuf_open(&fbuf, final_path.ptr, filebuf_flags(backend),
diff --git a/tests/odb/largefiles.c b/tests/odb/largefiles.c
index cd3651b..a5b982e 100644
--- a/tests/odb/largefiles.c
+++ b/tests/odb/largefiles.c
@@ -87,10 +87,10 @@ void test_odb_largefiles__streamread(void)
 	git_odb_stream *stream;
 	char buf[10240];
 	char hdr[64];
-	size_t len, total = 0;
+	size_t len, hdr_len, total = 0;
 	git_hash_ctx hash;
 	git_otype type;
-	int hdr_len, ret;
+	int ret;
 
 #ifndef GIT_ARCH_64
 	cl_skip();
@@ -108,7 +108,7 @@ void test_odb_largefiles__streamread(void)
 	cl_assert_equal_i(GIT_OBJ_BLOB, type);
 
 	cl_git_pass(git_hash_ctx_init(&hash));
-	hdr_len = git_odb__format_object_header(hdr, sizeof(hdr), len, type);
+	cl_git_pass(git_odb__format_object_header(&hdr_len, hdr, sizeof(hdr), len, type));
 
 	cl_git_pass(git_hash_update(&hash, hdr, hdr_len));