@matheusbsbs / matheus-bagliano / commits / 49c6b912e2

reftable/writer: ensure valid range for log's update_index

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@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Karthik Nayak committed Dec 6, 2024 at 14:13 UTC 49c6b912e2f4c49784d471f8c9364077c423dbf5

3 files changed +63 -4

reftable/writer.c

+12

@@ -412,6 +412,18 @@ int reftable_writer_add_log(struct reftable_writer *w,

412 if (log->value_type == REFTABLE_LOG_DELETION)

413 return reftable_writer_add_log_verbatim(w, log);

414

415 + /*

416 + * Verify only the upper limit of the update_index. Each reflog entry

417 + * is tied to a specific update_index. Entries in the reflog can be

418 + * replaced by adding a new entry with the same update_index,

419 + * effectively canceling the old one.

420 + *

421 + * Consequently, reflog updates may include update_index values lower

422 + * than the writer's min_update_index.

423 + */

424 + if (log->update_index > w->max_update_index)

425 + return REFTABLE_API_ERROR;

426 +

427 if (!log->refname)

428 return REFTABLE_API_ERROR;

429

t/unit-tests/t-reftable-readwrite.c

+45 -2

@@ -90,7 +90,7 @@ static void t_log_buffer_size(void)

90 int i;

91 struct reftable_log_record

92 log = { .refname = (char *) "refs/heads/master",

93 - .update_index = 0xa,

93 + .update_index = update_index,

94 .value_type = REFTABLE_LOG_UPDATE,

95 .value = { .update = {

96 .name = (char *) "Han-Wen Nienhuys",

@@ -127,7 +127,7 @@ static void t_log_overflow(void)

127 int err;

128 struct reftable_log_record log = {

129 .refname = (char *) "refs/heads/master",

130 - .update_index = 0xa,

130 + .update_index = update_index,

131 .value_type = REFTABLE_LOG_UPDATE,

132 .value = {

133 .update = {

@@ -151,6 +151,48 @@ static void t_log_overflow(void)

151 reftable_buf_release(&buf);

152 }

153

154 +static void t_log_write_limits(void)

155 +{

156 + struct reftable_write_options opts = { 0 };

157 + struct reftable_buf buf = REFTABLE_BUF_INIT;

158 + struct reftable_writer *w = t_reftable_strbuf_writer(&buf, &opts);

159 + struct reftable_log_record log = {

160 + .refname = (char *)"refs/head/master",

161 + .update_index = 0,

162 + .value_type = REFTABLE_LOG_UPDATE,

163 + .value = {

164 + .update = {

165 + .old_hash = { 1 },

166 + .new_hash = { 2 },

167 + .name = (char *)"Han-Wen Nienhuys",

168 + .email = (char *)"hanwen@google.com",

169 + .tz_offset = 100,

170 + .time = 0x5e430672,

171 + },

172 + },

173 + };

174 + int err;

175 +

176 + reftable_writer_set_limits(w, 1, 1);

177 +

178 + /* write with update_index (0) below set limits (1, 1) */

179 + err = reftable_writer_add_log(w, &log);

180 + check_int(err, ==, 0);

181 +

182 + /* write with update_index (1) in the set limits (1, 1) */

183 + log.update_index = 1;

184 + err = reftable_writer_add_log(w, &log);

185 + check_int(err, ==, 0);

186 +

187 + /* write with update_index (3) above set limits (1, 1) */

188 + log.update_index = 3;

189 + err = reftable_writer_add_log(w, &log);

190 + check_int(err, ==, REFTABLE_API_ERROR);

191 +

192 + reftable_writer_free(w);

193 + reftable_buf_release(&buf);

194 +}

195 +

196 static void t_log_write_read(void)

197 {

198 struct reftable_write_options opts = {

@@ -917,6 +959,7 @@ int cmd_main(int argc UNUSED, const char *argv[] UNUSED)

959 TEST(t_corrupt_table_empty(), "read-write on an empty table");

960 TEST(t_log_buffer_size(), "buffer extension for log compression");

961 TEST(t_log_overflow(), "log overflow returns expected error");

962 + TEST(t_log_write_limits(), "writer limits for writing log records");

963 TEST(t_log_write_read(), "read-write on log records");

964 TEST(t_log_zlib_corruption(), "reading corrupted log record returns expected error");

965 TEST(t_table_read_api(), "read on a table");

t/unit-tests/t-reftable-stack.c

+6 -2

@@ -770,8 +770,12 @@ static void t_reftable_stack_tombstone(void)

770 }

771

772 logs[i].refname = xstrdup(buf);

773 - /* update_index is part of the key. */

774 - logs[i].update_index = 42;

773 + /*

774 + * update_index is part of the key so should be constant.

775 + * The value itself should be less than the writer's upper

776 + * limit.

777 + */

778 + logs[i].update_index = 1;

779 if (i % 2 == 0) {

780 logs[i].value_type = REFTABLE_LOG_UPDATE;

781 t_reftable_set_hash(logs[i].value.update.new_hash, i,