| Message ID | e4c1cc58265ca7ae7b32b9faf41324883011d1a6.1642010868.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | reftable: avoid reading and writing empty keys | expand |
"Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@gmail.com> writes: > diff --git a/reftable/block_test.c b/reftable/block_test.c > index 4b3ea262dcb..5112ddbf468 100644 > --- a/reftable/block_test.c > +++ b/reftable/block_test.c > @@ -42,6 +42,11 @@ static void test_block_read_write(void) > header_off, hash_size(GIT_SHA1_FORMAT_ID)); > reftable_record_from_ref(&rec, &ref); > > + ref.refname = ""; > + ref.value_type = REFTABLE_REF_DELETION; > + n = block_writer_add(&bw, &rec); > + EXPECT(n == REFTABLE_API_ERROR); > + The preimage of this hunk has been invalidated by your 9c498398 (reftable: make reftable_record a tagged union, 2021-12-22). I see that the hn/reftable-coverity-fixes topic, which the commit is a part of, has been expecting a reroll since last year---are you plannning to rebuild that series after landing this series first?
On Fri, Jan 14, 2022 at 2:26 AM Junio C Hamano <gitster@pobox.com> wrote: > > + ref.refname = ""; > > + ref.value_type = REFTABLE_REF_DELETION; > > + n = block_writer_add(&bw, &rec); > > + EXPECT(n == REFTABLE_API_ERROR); > > + > > The preimage of this hunk has been invalidated by your 9c498398 > (reftable: make reftable_record a tagged union, 2021-12-22). > > I see that the hn/reftable-coverity-fixes topic, which the commit is > a part of, has been expecting a reroll since last year---are you I sent a reroll on Jan 12. As that has had more scrutiny, it's best if that lands first. This series is less urgent. > plannning to rebuild that series after landing this series first?
Han-Wen Nienhuys <hanwen@google.com> writes: >> I see that the hn/reftable-coverity-fixes topic, which the commit is >> a part of, has been expecting a reroll since last year---are you > > I sent a reroll on Jan 12. As that has had more scrutiny, it's best if > that lands first. This series is less urgent. Hmph, it probably was missed during pre-release freeze. https://lore.kernel.org/git/e16bf0c5212ae85daa0d6aa2c78d551824b542bd.1640199396.git.gitgitgadget@gmail.com/ not showing the updated ones did not help, either but stuff outside the upcoming release are not urgent right now, so it is OK. Thanks.
diff --git a/reftable/block.c b/reftable/block.c index 855e3f5c947..8725eaaf64f 100644 --- a/reftable/block.c +++ b/reftable/block.c @@ -88,8 +88,9 @@ uint8_t block_writer_type(struct block_writer *bw) return bw->buf[bw->header_off]; } -/* adds the reftable_record to the block. Returns -1 if it does not fit, 0 on - success */ +/* Adds the reftable_record to the block. Returns -1 if it does not fit, 0 on + success. Returns REFTABLE_API_ERROR if attempting to write a record with + empty key. */ int block_writer_add(struct block_writer *w, struct reftable_record *rec) { struct strbuf empty = STRBUF_INIT; @@ -105,8 +106,14 @@ int block_writer_add(struct block_writer *w, struct reftable_record *rec) int is_restart = 0; struct strbuf key = STRBUF_INIT; int n = 0; + int err = -1; reftable_record_key(rec, &key); + if (!key.len) { + err = REFTABLE_API_ERROR; + goto done; + } + n = reftable_encode_key(&is_restart, out, last, key, reftable_record_val_type(rec)); if (n < 0) @@ -118,16 +125,11 @@ int block_writer_add(struct block_writer *w, struct reftable_record *rec) goto done; string_view_consume(&out, n); - if (block_writer_register_restart(w, start.len - out.len, is_restart, - &key) < 0) - goto done; - - strbuf_release(&key); - return 0; - + err = block_writer_register_restart(w, start.len - out.len, is_restart, + &key); done: strbuf_release(&key); - return -1; + return err; } int block_writer_finish(struct block_writer *w) @@ -324,6 +326,9 @@ int block_iter_next(struct block_iter *it, struct reftable_record *rec) if (n < 0) return -1; + if (!key.len) + return REFTABLE_FORMAT_ERROR; + string_view_consume(&in, n); n = reftable_record_decode(rec, key, extra, in, it->br->hash_size); if (n < 0) @@ -350,6 +355,8 @@ int block_reader_first_key(struct block_reader *br, struct strbuf *key) int n = reftable_decode_key(key, &extra, empty, in); if (n < 0) return n; + if (!key->len) + return -1; return 0; } diff --git a/reftable/block_test.c b/reftable/block_test.c index 4b3ea262dcb..5112ddbf468 100644 --- a/reftable/block_test.c +++ b/reftable/block_test.c @@ -42,6 +42,11 @@ static void test_block_read_write(void) header_off, hash_size(GIT_SHA1_FORMAT_ID)); reftable_record_from_ref(&rec, &ref); + ref.refname = ""; + ref.value_type = REFTABLE_REF_DELETION; + n = block_writer_add(&bw, &rec); + EXPECT(n == REFTABLE_API_ERROR); + for (i = 0; i < N; i++) { char name[100]; uint8_t hash[GIT_SHA1_RAWSZ]; diff --git a/reftable/writer.c b/reftable/writer.c index 35c8649c9b7..e3c042b9d84 100644 --- a/reftable/writer.c +++ b/reftable/writer.c @@ -238,14 +238,13 @@ 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) { + if (err == -1) { /* we are writing into memory, so an error can only mean it * doesn't fit. */ err = REFTABLE_ENTRY_TOO_BIG_ERROR; goto done; } - err = 0; done: strbuf_release(&key); return err;