[PATCH 2/3] reftable: signal overflow

From: Han-Wen Nienhuys via GitGitGadget <gitgitgadget@gmail.com>
Date: Dec 23, 2021, 19:29 UTC
Message-ID: <53c1519525ff170bcc086de66cd82294d43bc69e.1640287790.git.gitgitgadget@gmail.com>
In-Reply-To: <pull.1164.git.git.1640287790.gitgitgadget@gmail.com>

From: Han-Wen Nienhuys <hanwen@google.com>

reflog entries have unbounded size. In theory, each log ('g') block in reftable can have an arbitrary size, so the format allows for arbitrarily sized reflog messages. However, in the implementation, we are not scaling the log blocks up with the message, and writing a large message fails.

This triggers a failure for reftable in t7006-pager.sh.

Until this is fixed more structurally, report an error from within the reftable library for easier debugging.

Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
---
 reftable/error.c          |  2 ++
 reftable/readwrite_test.c | 35 +++++++++++++++++++++++++++++++++++
 reftable/reftable-error.h |  4 ++++
 reftable/writer.c         |  3 +++
 4 files changed, 44 insertions(+)

diff --git a/reftable/error.c b/reftable/error.c
index f6f16def921..93941f21457 100644
--- a/reftable/error.c
+++ b/reftable/error.c
@@ -32,6 +32,8 @@ const char *reftable_error_str(int err)
 		return "wrote empty table";
 	case REFTABLE_REFNAME_ERROR:
 		return "invalid refname";
+	case REFTABLE_ENTRY_TOO_BIG_ERROR:
+		return "entry too large";
 	case -1:
 		return "general error";
 	default:
diff --git a/reftable/readwrite_test.c b/reftable/readwrite_test.c
index 5f6bcc2f775..70c7aedba2c 100644
--- a/reftable/readwrite_test.c
+++ b/reftable/readwrite_test.c
@@ -155,6 +155,40 @@ static void test_log_buffer_size(void)
 	strbuf_release(&buf);
 }
 
+static void test_log_overflow(void)
+{
+	struct strbuf buf = STRBUF_INIT;
+	char msg[256] = { 0 };
+	struct reftable_write_options opts = {
+		.block_size = ARRAY_SIZE(msg),
+	};
+	int err;
+	struct reftable_log_record
+		log = { .refname = "refs/heads/master",
+			.update_index = 0xa,
+			.value_type = REFTABLE_LOG_UPDATE,
+			.value = { .update = {
+					   .name = "Han-Wen Nienhuys",
+					   .email = "hanwen@google.com",
+					   .tz_offset = 100,
+					   .time = 0x5e430672,
+					   .message = msg,
+				   } } };
+	struct reftable_writer *w =
+		reftable_new_writer(&strbuf_add_void, &buf, &opts);
+
+	uint8_t hash1[GIT_SHA1_RAWSZ]  = {1}, hash2[GIT_SHA1_RAWSZ] = { 2 };
+
+	memset(msg, 'x', sizeof(msg) - 1);
+	log.value.update.old_hash = hash1;
+	log.value.update.new_hash = hash2;
+	reftable_writer_set_limits(w, update_index, update_index);
+	err = reftable_writer_add_log(w, &log);
+	EXPECT(err == REFTABLE_ENTRY_TOO_BIG_ERROR);
+	reftable_writer_free(w);
+	strbuf_release(&buf);
+}
+
 static void test_log_write_read(void)
 {
 	int N = 2;
@@ -648,5 +682,6 @@ int readwrite_test_main(int argc, const char *argv[])
 	RUN_TEST(test_table_refs_for_no_index);
 	RUN_TEST(test_table_refs_for_obj_index);
 	RUN_TEST(test_write_empty_table);
+	RUN_TEST(test_log_overflow);
 	return 0;
 }
diff --git a/reftable/reftable-error.h b/reftable/reftable-error.h
index 6f89bedf1a5..4c457aaaf89 100644
--- a/reftable/reftable-error.h
+++ b/reftable/reftable-error.h
@@ -53,6 +53,10 @@ enum reftable_error {
 
 	/* Invalid ref name. */
 	REFTABLE_REFNAME_ERROR = -10,
+
+	/* Entry does not fit. This can happen when writing outsize reflog
+	   messages. */
+	REFTABLE_ENTRY_TOO_BIG_ERROR = -11,
 };
 
 /* convert the numeric error code to a string. The string should not be
diff --git a/reftable/writer.c b/reftable/writer.c
index 3ca721e9f64..35c8649c9b7 100644
--- a/reftable/writer.c
+++ b/reftable/writer.c
@@ -239,6 +239,9 @@ static int writer_add_record(struct reftable_writer *w,
 	writer_reinit_block_writer(w, reftable_record_type(rec));
 	err = block_writer_add(w->block_writer, rec);
 	if (err < 0) {
+		/* we are writing into memory, so an error can only mean it
+		 * doesn't fit. */
+		err = REFTABLE_ENTRY_TOO_BIG_ERROR;
 		goto done;
 	}

-- 
gitgitgadget