reftable: avoid writing empty keys at the block layer

The public interface (reftable_writer) already ensures that keys are written in strictly increasing order, and an empty key by definition fails this check. However, by also enforcing this at the block layer, it is easier to verify that records (which are written into blocks) never have to consider the possibility of empty keys. Signed-off-by: Han-Wen Nienhuys <hanwen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Han-Wen Nienhuys committed Feb 21, 2022 at 18:46 UTC 45c2fcc2a0659e42c7e4f21f0b97393df261ca51
3 files changed +23 -12
reftable/block.c
+17 -10
@@ -88,8 +88,9 @@ uint8_t block_writer_type(struct block_writer *bw)
88 return bw->buf[bw->header_off];
89 }
90
91 -/* adds the reftable_record to the block. Returns -1 if it does not fit, 0 on
92 - success */
91 +/* Adds the reftable_record to the block. Returns -1 if it does not fit, 0 on
92 + success. Returns REFTABLE_API_ERROR if attempting to write a record with
93 + empty key. */
94 int block_writer_add(struct block_writer *w, struct reftable_record *rec)
95 {
96 struct strbuf empty = STRBUF_INIT;
@@ -105,8 +106,14 @@ int block_writer_add(struct block_writer *w, struct reftable_record *rec)
106 int is_restart = 0;
107 struct strbuf key = STRBUF_INIT;
108 int n = 0;
109 + int err = -1;
110
111 reftable_record_key(rec, &key);
112 + if (!key.len) {
113 + err = REFTABLE_API_ERROR;
114 + goto done;
115 + }
116 +
117 n = reftable_encode_key(&is_restart, out, last, key,
118 reftable_record_val_type(rec));
119 if (n < 0)
@@ -118,16 +125,11 @@ int block_writer_add(struct block_writer *w, struct reftable_record *rec)
125 goto done;
126 string_view_consume(&out, n);
127
121 - if (block_writer_register_restart(w, start.len - out.len, is_restart,
122 - &key) < 0)
123 - goto done;
124 -
125 - strbuf_release(&key);
126 - return 0;
127 -
128 + err = block_writer_register_restart(w, start.len - out.len, is_restart,
129 + &key);
130 done:
131 strbuf_release(&key);
130 - return -1;
132 + return err;
133 }
134
135 int block_writer_finish(struct block_writer *w)
@@ -332,6 +334,9 @@ int block_iter_next(struct block_iter *it, struct reftable_record *rec)
334 if (n < 0)
335 return -1;
336
337 + if (!key.len)
338 + return REFTABLE_FORMAT_ERROR;
339 +
340 string_view_consume(&in, n);
341 n = reftable_record_decode(rec, key, extra, in, it->br->hash_size);
342 if (n < 0)
@@ -358,6 +363,8 @@ int block_reader_first_key(struct block_reader *br, struct strbuf *key)
363 int n = reftable_decode_key(key, &extra, empty, in);
364 if (n < 0)
365 return n;
366 + if (!key->len)
367 + return REFTABLE_FORMAT_ERROR;
368
369 return 0;
370 }
reftable/block_test.c
+5
@@ -42,6 +42,11 @@ static void test_block_read_write(void)
42 block_writer_init(&bw, BLOCK_TYPE_REF, block.data, block_size,
43 header_off, hash_size(GIT_SHA1_FORMAT_ID));
44
45 + rec.u.ref.refname = "";
46 + rec.u.ref.value_type = REFTABLE_REF_DELETION;
47 + n = block_writer_add(&bw, &rec);
48 + EXPECT(n == REFTABLE_API_ERROR);
49 +
50 for (i = 0; i < N; i++) {
51 char name[100];
52 uint8_t hash[GIT_SHA1_RAWSZ];
reftable/writer.c
+1 -2
@@ -240,14 +240,13 @@ static int writer_add_record(struct reftable_writer *w,
240
241 writer_reinit_block_writer(w, reftable_record_type(rec));
242 err = block_writer_add(w->block_writer, rec);
243 - if (err < 0) {
243 + if (err == -1) {
244 /* we are writing into memory, so an error can only mean it
245 * doesn't fit. */
246 err = REFTABLE_ENTRY_TOO_BIG_ERROR;
247 goto done;
248 }
249
250 - err = 0;
250 done:
251 strbuf_release(&key);
252 return err;