From: Martin Matuska Date: Fri, 6 Dec 2019 10:25:19 +0000 (+0100) Subject: Implement private state logic for write filters X-Git-Tag: v3.4.1~10 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6f4fceb714868f2ddbccef4871acc1670e45fc03;p=thirdparty%2Flibarchive.git Implement private state logic for write filters This ensures that filters may be opened and closed only once and __archive_write_filter() may be called only on an open filter. Refactor filter open code and move logic to archive_write.c Fixes #351 --- diff --git a/libarchive/archive_write.c b/libarchive/archive_write.c index 1c40e9769..e7a973ae4 100644 --- a/libarchive/archive_write.c +++ b/libarchive/archive_write.c @@ -212,6 +212,7 @@ __archive_write_allocate_filter(struct archive *_a) f = calloc(1, sizeof(*f)); f->archive = _a; + f->state = ARCHIVE_WRITE_FILTER_STATE_NEW; if (a->filter_first == NULL) a->filter_first = f; else @@ -228,6 +229,9 @@ __archive_write_filter(struct archive_write_filter *f, const void *buff, size_t length) { int r; + /* Never write to non-open filters */ + if (f->state != ARCHIVE_WRITE_FILTER_STATE_OPEN) + return(ARCHIVE_FATAL); if (length == 0) return(ARCHIVE_OK); if (f->write == NULL) @@ -240,30 +244,67 @@ __archive_write_filter(struct archive_write_filter *f, } /* - * Open a filter. + * Recursive function for opening the filter chain + * Last filter is opened first */ -int +static int __archive_write_open_filter(struct archive_write_filter *f) { - if (f->open == NULL) + int ret; + + ret = ARCHIVE_OK; + if (f->next_filter != NULL) + ret = __archive_write_open_filter(f->next_filter); + if (ret != ARCHIVE_OK) + return (ret); + if (f->state != ARCHIVE_WRITE_FILTER_STATE_NEW) + return (ARCHIVE_FATAL); + if (f->open == NULL) { + f->state = ARCHIVE_WRITE_FILTER_STATE_OPEN; return (ARCHIVE_OK); - return (f->open)(f); + } + ret = (f->open)(f); + if (ret == ARCHIVE_OK) + f->state = ARCHIVE_WRITE_FILTER_STATE_OPEN; + else + f->state = ARCHIVE_WRITE_FILTER_STATE_FATAL; + return (ret); } /* - * Close a filter. + * Open all filters */ -int -__archive_write_close_filter(struct archive_write *a) +static int +__archive_write_filters_open(struct archive_write *a) +{ + return (__archive_write_open_filter(a->filter_first)); +} + +/* + * Close all filtes + */ +static int +__archive_write_filters_close(struct archive_write *a) { struct archive_write_filter *f; int ret, ret1; ret = ARCHIVE_OK; for (f = a->filter_first; f != NULL; f = f->next_filter) { - if (f->close != NULL) { - ret1 = (f->close)(f); - if (ret1 < ret) - ret = ret1; + /* Do not close filters that are not open */ + if (f->state == ARCHIVE_WRITE_FILTER_STATE_OPEN) { + if (f->close != NULL) { + ret1 = (f->close)(f); + if (ret1 < ret) + ret = ret1; + if (ret1 == ARCHIVE_OK) { + f->state = + ARCHIVE_WRITE_FILTER_STATE_CLOSED; + } else { + f->state = + ARCHIVE_WRITE_FILTER_STATE_FATAL; + } + } else + f->state = ARCHIVE_WRITE_FILTER_STATE_CLOSED; } } return (ret); @@ -446,7 +487,7 @@ archive_write_client_close(struct archive_write_filter *f) free(state->buffer); free(state); /* Clear the close handler myself not to be called again. */ - f->close = NULL; + f->state = ARCHIVE_WRITE_FILTER_STATE_CLOSED; a->client_data = NULL; /* Clear passphrase. */ if (a->passphrase != NULL) { @@ -483,9 +524,9 @@ archive_write_open(struct archive *_a, void *client_data, client_filter->write = archive_write_client_write; client_filter->close = archive_write_client_close; - ret = __archive_write_open_filter(a->filter_first); + ret = __archive_write_filters_open(a); if (ret < ARCHIVE_WARN) { - r1 = __archive_write_close_filter(a); + r1 = __archive_write_filters_close(a); __archive_write_filters_free(_a); return (r1 < ret ? r1 : ret); } @@ -528,7 +569,7 @@ _archive_write_close(struct archive *_a) } /* Finish the compression and close the stream. */ - r1 = __archive_write_close_filter(a); + r1 = __archive_write_filters_close(a); if (r1 < r) r = r1; diff --git a/libarchive/archive_write_add_filter_b64encode.c b/libarchive/archive_write_add_filter_b64encode.c index fe4ae247b..87fdb73ec 100644 --- a/libarchive/archive_write_add_filter_b64encode.c +++ b/libarchive/archive_write_add_filter_b64encode.c @@ -149,11 +149,6 @@ archive_filter_b64encode_open(struct archive_write_filter *f) { struct private_b64encode *state = (struct private_b64encode *)f->data; size_t bs = 65536, bpb; - int ret; - - ret = __archive_write_open_filter(f->next_filter); - if (ret != ARCHIVE_OK) - return (ret); if (f->archive->magic == ARCHIVE_WRITE_MAGIC) { /* Buffer size should be a multiple number of the of bytes diff --git a/libarchive/archive_write_add_filter_bzip2.c b/libarchive/archive_write_add_filter_bzip2.c index 13f88e986..7001e9c6b 100644 --- a/libarchive/archive_write_add_filter_bzip2.c +++ b/libarchive/archive_write_add_filter_bzip2.c @@ -167,10 +167,6 @@ archive_compressor_bzip2_open(struct archive_write_filter *f) struct private_data *data = (struct private_data *)f->data; int ret; - ret = __archive_write_open_filter(f->next_filter); - if (ret != 0) - return (ret); - if (data->compressed == NULL) { size_t bs = 65536, bpb; if (f->archive->magic == ARCHIVE_WRITE_MAGIC) { diff --git a/libarchive/archive_write_add_filter_compress.c b/libarchive/archive_write_add_filter_compress.c index 285dfc2de..d404fae7d 100644 --- a/libarchive/archive_write_add_filter_compress.c +++ b/libarchive/archive_write_add_filter_compress.c @@ -146,17 +146,12 @@ archive_write_add_filter_compress(struct archive *_a) static int archive_compressor_compress_open(struct archive_write_filter *f) { - int ret; struct private_data *state; size_t bs = 65536, bpb; f->code = ARCHIVE_FILTER_COMPRESS; f->name = "compress"; - ret = __archive_write_open_filter(f->next_filter); - if (ret != ARCHIVE_OK) - return (ret); - state = (struct private_data *)calloc(1, sizeof(*state)); if (state == NULL) { archive_set_error(f->archive, ENOMEM, diff --git a/libarchive/archive_write_add_filter_gzip.c b/libarchive/archive_write_add_filter_gzip.c index 5deb17e24..8670d5ca7 100644 --- a/libarchive/archive_write_add_filter_gzip.c +++ b/libarchive/archive_write_add_filter_gzip.c @@ -184,10 +184,6 @@ archive_compressor_gzip_open(struct archive_write_filter *f) struct private_data *data = (struct private_data *)f->data; int ret; - ret = __archive_write_open_filter(f->next_filter); - if (ret != ARCHIVE_OK) - return (ret); - if (data->compressed == NULL) { size_t bs = 65536, bpb; if (f->archive->magic == ARCHIVE_WRITE_MAGIC) { diff --git a/libarchive/archive_write_add_filter_lz4.c b/libarchive/archive_write_add_filter_lz4.c index c9bced55d..cf19fadd5 100644 --- a/libarchive/archive_write_add_filter_lz4.c +++ b/libarchive/archive_write_add_filter_lz4.c @@ -223,16 +223,11 @@ static int archive_filter_lz4_open(struct archive_write_filter *f) { struct private_data *data = (struct private_data *)f->data; - int ret; size_t required_size; static size_t const bkmap[] = { 64 * 1024, 256 * 1024, 1 * 1024 * 1024, 4 * 1024 * 1024 }; size_t pre_block_size; - ret = __archive_write_open_filter(f->next_filter); - if (ret != 0) - return (ret); - if (data->block_maximum_size < 4) data->block_size = bkmap[0]; else diff --git a/libarchive/archive_write_add_filter_lzop.c b/libarchive/archive_write_add_filter_lzop.c index 6a2dd86c5..3bd9062e4 100644 --- a/libarchive/archive_write_add_filter_lzop.c +++ b/libarchive/archive_write_add_filter_lzop.c @@ -228,11 +228,6 @@ static int archive_write_lzop_open(struct archive_write_filter *f) { struct write_lzop *data = (struct write_lzop *)f->data; - int ret; - - ret = __archive_write_open_filter(f->next_filter); - if (ret != ARCHIVE_OK) - return (ret); switch (data->compression_level) { case 1: diff --git a/libarchive/archive_write_add_filter_program.c b/libarchive/archive_write_add_filter_program.c index 74588d3a5..a4bc1d90e 100644 --- a/libarchive/archive_write_add_filter_program.c +++ b/libarchive/archive_write_add_filter_program.c @@ -212,11 +212,6 @@ __archive_write_program_open(struct archive_write_filter *f, struct archive_write_program_data *data, const char *cmd) { pid_t child; - int ret; - - ret = __archive_write_open_filter(f->next_filter); - if (ret != ARCHIVE_OK) - return (ret); if (data->child_buf == NULL) { data->child_buf_len = 65536; diff --git a/libarchive/archive_write_add_filter_uuencode.c b/libarchive/archive_write_add_filter_uuencode.c index 92fab6bbf..1ad458921 100644 --- a/libarchive/archive_write_add_filter_uuencode.c +++ b/libarchive/archive_write_add_filter_uuencode.c @@ -138,11 +138,6 @@ archive_filter_uuencode_open(struct archive_write_filter *f) { struct private_uuencode *state = (struct private_uuencode *)f->data; size_t bs = 65536, bpb; - int ret; - - ret = __archive_write_open_filter(f->next_filter); - if (ret != ARCHIVE_OK) - return (ret); if (f->archive->magic == ARCHIVE_WRITE_MAGIC) { /* Buffer size should be a multiple number of the of bytes diff --git a/libarchive/archive_write_add_filter_xz.c b/libarchive/archive_write_add_filter_xz.c index 557cc77e4..8c1ebb805 100644 --- a/libarchive/archive_write_add_filter_xz.c +++ b/libarchive/archive_write_add_filter_xz.c @@ -309,10 +309,6 @@ archive_compressor_xz_open(struct archive_write_filter *f) struct private_data *data = f->data; int ret; - ret = __archive_write_open_filter(f->next_filter); - if (ret != ARCHIVE_OK) - return (ret); - if (data->compressed == NULL) { size_t bs = 65536, bpb; if (f->archive->magic == ARCHIVE_WRITE_MAGIC) { diff --git a/libarchive/archive_write_add_filter_zstd.c b/libarchive/archive_write_add_filter_zstd.c index cc57d3fef..4c91551ed 100644 --- a/libarchive/archive_write_add_filter_zstd.c +++ b/libarchive/archive_write_add_filter_zstd.c @@ -172,11 +172,6 @@ static int archive_compressor_zstd_open(struct archive_write_filter *f) { struct private_data *data = (struct private_data *)f->data; - int ret; - - ret = __archive_write_open_filter(f->next_filter); - if (ret != ARCHIVE_OK) - return (ret); if (data->out.dst == NULL) { size_t bs = ZSTD_CStreamOutSize(), bpb; diff --git a/libarchive/archive_write_private.h b/libarchive/archive_write_private.h index 234893e39..1c182f136 100644 --- a/libarchive/archive_write_private.h +++ b/libarchive/archive_write_private.h @@ -38,6 +38,11 @@ #include "archive_string.h" #include "archive_private.h" +#define ARCHIVE_WRITE_FILTER_STATE_NEW 1U +#define ARCHIVE_WRITE_FILTER_STATE_OPEN 2U +#define ARCHIVE_WRITE_FILTER_STATE_CLOSED 4U +#define ARCHIVE_WRITE_FILTER_STATE_FATAL 0x8000U + struct archive_write; struct archive_write_filter { @@ -55,6 +60,7 @@ struct archive_write_filter { int code; int bytes_per_block; int bytes_in_last_block; + int state; }; #if ARCHIVE_VERSION < 4000000 @@ -66,8 +72,6 @@ struct archive_write_filter *__archive_write_allocate_filter(struct archive *); int __archive_write_output(struct archive_write *, const void *, size_t); int __archive_write_nulls(struct archive_write *, size_t); int __archive_write_filter(struct archive_write_filter *, const void *, size_t); -int __archive_write_open_filter(struct archive_write_filter *); -int __archive_write_close_filter(struct archive_write *); struct archive_write { struct archive archive; diff --git a/libarchive/test/test_open_failure.c b/libarchive/test/test_open_failure.c index 845486cf9..5316a872b 100644 --- a/libarchive/test/test_open_failure.c +++ b/libarchive/test/test_open_failure.c @@ -160,11 +160,11 @@ DEFINE_TEST(test_open_failure) archive_write_open(a, &private, my_open, my_write, my_close)); assertEqualInt(1, private.open_called); assertEqualInt(0, private.write_called); - assertEqualInt(1, private.close_called); + assertEqualInt(0, private.close_called); assertEqualInt(ARCHIVE_OK, archive_write_free(a)); assertEqualInt(1, private.open_called); assertEqualInt(0, private.write_called); - assertEqualInt(1, private.close_called); + assertEqualInt(0, private.close_called); memset(&private, 0, sizeof(private)); private.magic = MAGIC; @@ -177,11 +177,11 @@ DEFINE_TEST(test_open_failure) archive_write_open(a, &private, my_open, my_write, my_close)); assertEqualInt(1, private.open_called); assertEqualInt(0, private.write_called); - assertEqualInt(1, private.close_called); + assertEqualInt(0, private.close_called); assertEqualInt(ARCHIVE_OK, archive_write_free(a)); assertEqualInt(1, private.open_called); assertEqualInt(0, private.write_called); - assertEqualInt(1, private.close_called); + assertEqualInt(0, private.close_called); memset(&private, 0, sizeof(private)); private.magic = MAGIC; @@ -193,11 +193,11 @@ DEFINE_TEST(test_open_failure) archive_write_open(a, &private, my_open, my_write, my_close)); assertEqualInt(1, private.open_called); assertEqualInt(0, private.write_called); - assertEqualInt(1, private.close_called); + assertEqualInt(0, private.close_called); assertEqualInt(ARCHIVE_OK, archive_write_free(a)); assertEqualInt(1, private.open_called); assertEqualInt(0, private.write_called); - assertEqualInt(1, private.close_called); + assertEqualInt(0, private.close_called); memset(&private, 0, sizeof(private)); private.magic = MAGIC; @@ -209,10 +209,10 @@ DEFINE_TEST(test_open_failure) archive_write_open(a, &private, my_open, my_write, my_close)); assertEqualInt(1, private.open_called); assertEqualInt(0, private.write_called); - assertEqualInt(1, private.close_called); + assertEqualInt(0, private.close_called); assertEqualInt(ARCHIVE_OK, archive_write_free(a)); assertEqualInt(1, private.open_called); assertEqualInt(0, private.write_called); - assertEqualInt(1, private.close_called); + assertEqualInt(0, private.close_called); }