Each reftable addition has an associated update_index. While writing
refs, the update_index is verified to be within the range of the
reftable writer, i.e. `writer.min_update_index < ref.update_index` and
`writer.max_update_index > ref.update_index`.
The corresponding check for reflogs in `reftable_writer_add_log` is
however missing. Add a similar check, but only check for the upper
limit. This is because reflogs are treated a bit differently than refs.
Each reflog entry in reftable has an associated update_index and we also
allow expiring entries in the middle, which is done by simply writing a
new reflog entry with the same update_index. This means, writing reflog
entries with update_index lesser than the writer's update_index is an
expected scenario.
Add a new unit test to check for the limits and fix some of the existing
tests, which were setting arbitrary values for the update_index by
ensuring they stay within the now checked limits.
Signed-off-by: Karthik Nayak <karthik.188@xxxxxxxxx>
---
reftable/writer.c | 12 ++++++++++
t/unit-tests/t-reftable-readwrite.c | 44 +++++++++++++++++++++++++++++++++++--
t/unit-tests/t-reftable-stack.c | 8 +++++--
3 files changed, 60 insertions(+), 4 deletions(-)
diff --git a/reftable/writer.c b/reftable/writer.c
index fd136794d5a27b33b5017f36fbd6b095ab8dac5b..f87086777cd20a9890a63f10c5d6932310dd5610 100644
--- a/reftable/writer.c
+++ b/reftable/writer.c
@@ -412,6 +412,18 @@ int reftable_writer_add_log(struct reftable_writer *w,
if (log->value_type == REFTABLE_LOG_DELETION)
return reftable_writer_add_log_verbatim(w, log);
+ /*
+ * Verify only the upper limit of the update_index. Each reflog entry
+ * is tied to a specific update_index. Entries in the reflog can be
+ * replaced by adding a new entry with the same update_index,
+ * effectively canceling the old one.
+ *
+ * Consequently, reflog updates may include update_index values lower
+ * than the writer's min_update_index.
+ */
+ if (log->update_index > w->max_update_index)
+ return REFTABLE_API_ERROR;
+
if (!log->refname)
return REFTABLE_API_ERROR;
diff --git a/t/unit-tests/t-reftable-readwrite.c b/t/unit-tests/t-reftable-readwrite.c
index d279b86df0aeda11b3fb4d2c15803760ae394941..5ad1c72f6901abcfe7fdc6c3e69e26b58d0013a6 100644
--- a/t/unit-tests/t-reftable-readwrite.c
+++ b/t/unit-tests/t-reftable-readwrite.c
@@ -90,7 +90,7 @@ static void t_log_buffer_size(void)
int i;
struct reftable_log_record
log = { .refname = (char *) "refs/heads/master",
- .update_index = 0xa,
+ .update_index = update_index,
.value_type = REFTABLE_LOG_UPDATE,
.value = { .update = {
.name = (char *) "Han-Wen Nienhuys",
@@ -127,7 +127,7 @@ static void t_log_overflow(void)
int err;
struct reftable_log_record log = {
.refname = (char *) "refs/heads/master",
- .update_index = 0xa,
+ .update_index = update_index,
.value_type = REFTABLE_LOG_UPDATE,
.value = {
.update = {
@@ -151,6 +151,45 @@ static void t_log_overflow(void)
reftable_buf_release(&buf);
}
+static void t_log_write_limits(void)
+{
+ struct reftable_write_options opts = { 0 };
+ struct reftable_buf buf = REFTABLE_BUF_INIT;
+ struct reftable_writer *w = t_reftable_strbuf_writer(&buf, &opts);
+ struct reftable_log_record log = {
+ .refname = (char *)"refs/head/master",
+ .update_index = 1,
+ .value_type = REFTABLE_LOG_UPDATE,
+ .value = {
+ .update = {
+ .old_hash = { 1 },
+ .new_hash = { 2 },
+ .name = (char *)"Han-Wen Nienhuys",
+ .email = (char *)"hanwen@xxxxxxxxxx",
+ .tz_offset = 100,
+ .time = 0x5e430672,
+ },
+ },
+ };
+ int err;
+
+ reftable_writer_set_limits(w, 1, 2);
+
+ err = reftable_writer_add_log(w, &log);
+ check_int(err, ==, 0);
+
+ log.update_index = 2;
+ err = reftable_writer_add_log(w, &log);
+ check_int(err, ==, 0);
+
+ log.update_index = 3;
+ err = reftable_writer_add_log(w, &log);
+ check_int(err, ==, REFTABLE_API_ERROR);
+
+ reftable_writer_free(w);
+ reftable_buf_release(&buf);
+}
+
static void t_log_write_read(void)
{
struct reftable_write_options opts = {
@@ -917,6 +956,7 @@ int cmd_main(int argc UNUSED, const char *argv[] UNUSED)
TEST(t_corrupt_table_empty(), "read-write on an empty table");
TEST(t_log_buffer_size(), "buffer extension for log compression");
TEST(t_log_overflow(), "log overflow returns expected error");
+ TEST(t_log_write_limits(), "writer limits for writing log records");
TEST(t_log_write_read(), "read-write on log records");
TEST(t_log_zlib_corruption(), "reading corrupted log record returns expected error");
TEST(t_table_read_api(), "read on a table");
diff --git a/t/unit-tests/t-reftable-stack.c b/t/unit-tests/t-reftable-stack.c
index 72f6747064f62106e87c9a90e5fe315139d46e60..52b81475c36aa94ff09f3bf77a7d23992957deb4 100644
--- a/t/unit-tests/t-reftable-stack.c
+++ b/t/unit-tests/t-reftable-stack.c
@@ -770,8 +770,12 @@ static void t_reftable_stack_tombstone(void)
}
logs[i].refname = xstrdup(buf);
- /* update_index is part of the key. */
- logs[i].update_index = 42;
+ /*
+ * update_index is part of the key so should be constant.
+ * The value itself should be less than the writer's upper
+ * limit.
+ */
+ logs[i].update_index = 1;
if (i % 2 == 0) {
logs[i].value_type = REFTABLE_LOG_UPDATE;
t_reftable_set_hash(logs[i].value.update.new_hash, i,
---
---
base-commit: cc01bad4a9f566cf4453c7edd6b433851b0835e2
change-id: 20241120-424-reftable-writer-add-check-for-limits-01b1fb703528
Thanks
- Karthik
