From 019bd340820463b2ce2c006c45b6c87fefdd551c Mon Sep 17 00:00:00 2001 From: Han-Wen Nienhuys Date: Thu, 23 Dec 2021 19:29:48 +0000 Subject: [PATCH 1/3] reftable: fix typo in header Signed-off-by: Han-Wen Nienhuys Signed-off-by: Junio C Hamano --- reftable/block.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/reftable/block.h b/reftable/block.h index e207706a64..87c77539b5 100644 --- a/reftable/block.h +++ b/reftable/block.h @@ -21,7 +21,7 @@ struct block_writer { uint8_t *buf; uint32_t block_size; - /* Offset ofof the global header. Nonzero in the first block only. */ + /* Offset of the global header. Nonzero in the first block only. */ uint32_t header_off; /* How often to restart keys. */ From 0dd44584abf3fee0ba19b5edf856be2a79974228 Mon Sep 17 00:00:00 2001 From: Han-Wen Nienhuys Date: Thu, 23 Dec 2021 19:29:49 +0000 Subject: [PATCH 2/3] reftable: signal overflow 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 Signed-off-by: Junio C Hamano --- 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 f6f16def92..93941f2145 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 5f6bcc2f77..70c7aedba2 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 6f89bedf1a..4c457aaaf8 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 3ca721e9f6..35c8649c9b 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; } From cd1799dea097cee064db1d09966fee88078476e9 Mon Sep 17 00:00:00 2001 From: Han-Wen Nienhuys Date: Thu, 23 Dec 2021 19:29:50 +0000 Subject: [PATCH 3/3] reftable: support preset file mode for writing Create files with mode 0666, so umask works as intended. Provides an override, which is useful to support shared repos (test t1301-shared-repo.sh). Signed-off-by: Han-Wen Nienhuys Signed-off-by: Junio C Hamano --- reftable/reftable-writer.h | 3 +++ reftable/stack.c | 30 ++++++++++++++++++++++++------ reftable/stack_test.c | 33 +++++++++++++++++++++++++++++---- 3 files changed, 56 insertions(+), 10 deletions(-) diff --git a/reftable/reftable-writer.h b/reftable/reftable-writer.h index af36462ced..a560dc1725 100644 --- a/reftable/reftable-writer.h +++ b/reftable/reftable-writer.h @@ -35,6 +35,9 @@ struct reftable_write_options { */ uint32_t hash_id; + /* Default mode for creating files. If unset, use 0666 (+umask) */ + unsigned int default_permissions; + /* boolean: do not check ref names for validity or dir/file conflicts. */ unsigned skip_name_check : 1; diff --git a/reftable/stack.c b/reftable/stack.c index df5021ebf0..56bf5f2d84 100644 --- a/reftable/stack.c +++ b/reftable/stack.c @@ -469,7 +469,7 @@ static int reftable_stack_init_addition(struct reftable_addition *add, strbuf_addstr(&add->lock_file_name, ".lock"); add->lock_file_fd = open(add->lock_file_name.buf, - O_EXCL | O_CREAT | O_WRONLY, 0644); + O_EXCL | O_CREAT | O_WRONLY, 0666); if (add->lock_file_fd < 0) { if (errno == EEXIST) { err = REFTABLE_LOCK_ERROR; @@ -478,6 +478,13 @@ static int reftable_stack_init_addition(struct reftable_addition *add, } goto done; } + if (st->config.default_permissions) { + if (chmod(add->lock_file_name.buf, st->config.default_permissions) < 0) { + err = REFTABLE_IO_ERROR; + goto done; + } + } + err = stack_uptodate(st); if (err < 0) goto done; @@ -644,7 +651,12 @@ int reftable_addition_add(struct reftable_addition *add, err = REFTABLE_IO_ERROR; goto done; } - + if (add->stack->config.default_permissions) { + if (chmod(temp_tab_file_name.buf, add->stack->config.default_permissions)) { + err = REFTABLE_IO_ERROR; + goto done; + } + } wr = reftable_new_writer(reftable_fd_write, &tab_fd, &add->stack->config); err = write_table(wr, arg); @@ -900,7 +912,7 @@ static int stack_compact_range(struct reftable_stack *st, int first, int last, strbuf_addstr(&lock_file_name, ".lock"); lock_file_fd = - open(lock_file_name.buf, O_EXCL | O_CREAT | O_WRONLY, 0644); + open(lock_file_name.buf, O_EXCL | O_CREAT | O_WRONLY, 0666); if (lock_file_fd < 0) { if (errno == EEXIST) { err = 1; @@ -931,8 +943,8 @@ static int stack_compact_range(struct reftable_stack *st, int first, int last, strbuf_addstr(&subtab_lock, ".lock"); sublock_file_fd = open(subtab_lock.buf, - O_EXCL | O_CREAT | O_WRONLY, 0644); - if (sublock_file_fd > 0) { + O_EXCL | O_CREAT | O_WRONLY, 0666); + if (sublock_file_fd >= 0) { close(sublock_file_fd); } else if (sublock_file_fd < 0) { if (errno == EEXIST) { @@ -967,7 +979,7 @@ static int stack_compact_range(struct reftable_stack *st, int first, int last, goto done; lock_file_fd = - open(lock_file_name.buf, O_EXCL | O_CREAT | O_WRONLY, 0644); + open(lock_file_name.buf, O_EXCL | O_CREAT | O_WRONLY, 0666); if (lock_file_fd < 0) { if (errno == EEXIST) { err = 1; @@ -977,6 +989,12 @@ static int stack_compact_range(struct reftable_stack *st, int first, int last, goto done; } have_lock = 1; + if (st->config.default_permissions) { + if (chmod(lock_file_name.buf, st->config.default_permissions) < 0) { + err = REFTABLE_IO_ERROR; + goto done; + } + } format_name(&new_table_name, st->readers[first]->min_update_index, st->readers[last]->max_update_index); diff --git a/reftable/stack_test.c b/reftable/stack_test.c index eb0b7228b0..f4c743db80 100644 --- a/reftable/stack_test.c +++ b/reftable/stack_test.c @@ -17,6 +17,7 @@ license that can be found in the LICENSE file or at #include "record.h" #include "test_framework.h" #include "reftable-tests.h" +#include "reader.h" #include #include @@ -138,8 +139,11 @@ static int write_test_log(struct reftable_writer *wr, void *arg) static void test_reftable_stack_add_one(void) { char *dir = get_tmp_dir(__LINE__); - - struct reftable_write_options cfg = { 0 }; + struct strbuf scratch = STRBUF_INIT; + int mask = umask(002); + struct reftable_write_options cfg = { + .default_permissions = 0660, + }; struct reftable_stack *st = NULL; int err; struct reftable_ref_record ref = { @@ -149,8 +153,7 @@ static void test_reftable_stack_add_one(void) .value.symref = "master", }; struct reftable_ref_record dest = { NULL }; - - + struct stat stat_result = { 0 }; err = reftable_new_stack(&st, dir, cfg); EXPECT_ERR(err); @@ -160,6 +163,7 @@ static void test_reftable_stack_add_one(void) err = reftable_stack_read_ref(st, ref.refname, &dest); EXPECT_ERR(err); EXPECT(0 == strcmp("master", dest.value.symref)); + EXPECT(st->readers_len > 0); printf("testing print functionality:\n"); err = reftable_stack_print_directory(dir, GIT_SHA1_FORMAT_ID); @@ -168,9 +172,30 @@ static void test_reftable_stack_add_one(void) err = reftable_stack_print_directory(dir, GIT_SHA256_FORMAT_ID); EXPECT(err == REFTABLE_FORMAT_ERROR); +#ifndef GIT_WINDOWS_NATIVE + strbuf_addstr(&scratch, dir); + strbuf_addstr(&scratch, "/tables.list"); + err = stat(scratch.buf, &stat_result); + EXPECT(!err); + EXPECT((stat_result.st_mode & 0777) == cfg.default_permissions); + + strbuf_reset(&scratch); + strbuf_addstr(&scratch, dir); + strbuf_addstr(&scratch, "/"); + /* do not try at home; not an external API for reftable. */ + strbuf_addstr(&scratch, st->readers[0]->name); + err = stat(scratch.buf, &stat_result); + EXPECT(!err); + EXPECT((stat_result.st_mode & 0777) == cfg.default_permissions); +#else + (void) stat_result; +#endif + reftable_ref_record_release(&dest); reftable_stack_destroy(st); + strbuf_release(&scratch); clear_dir(dir); + umask(mask); } static void test_reftable_stack_uptodate(void)