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 1c40e97697f0e03947b0bef06d5c9fe133ade4ca..e7a973ae41a70c71b84413f18d1499b6a1795d0f 100644 (file)
--- 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 fe4ae247b0575c69deebcb86dd0026c52c084dd2..87fdb73ecb09c608b6a1e458fa3cc41b4c5c6750 100644 (file)
--- 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 13f88e986c21ea97659b482e2fb01c570bcb7e12..7001e9c6b30913cfaaa23de74c2ee1bdc5999bb9 100644 (file)
--- 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 285dfc2deef9c64cd35ae4151aab38452ea75595..d404fae7dba48b9fddf986dcded5a7e2672e5b39 100644 (file)
--- 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 5deb17e24f432beb6e17e529176651e53b13765c..8670d5ca7403eae67243b93c876a77a9ce0afd9c 100644 (file)
--- 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 c9bced55da2eb12e5330ae8c750a2bec03842660..cf19fadd56339d89144575f76b156c7d66776b83 100644 (file)
--- 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 6a2dd86c57c8018438d5bf741dbc600b7d3ebaca..3bd9062e4d327a1490e95bd2a77a1c5ac635cca1 100644 (file)
--- 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 74588d3a5ea072a946119cf84de28efd295f7fdb..a4bc1d90eda8a81ab9979f2029b47dfe1ce2c415 100644 (file)
--- 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 92fab6bbf5d4a7938dd5c7d08a7b3ece98162bc5..1ad4589219287a3fac97c606e58cfc00c172c45e 100644 (file)
--- 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 557cc77e4077a5980663667ff7d1a9dceb4ca40b..8c1ebb805b10b973be32b008d13e378ed2f94ad1 100644 (file)
--- 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 cc57d3fef8e7f4e507fcc58afac6be6855e5f69f..4c91551ed3e132e9daa09cb0580bcec64a509f27 100644 (file)
--- 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 234893e39f014b9fef1e83128b7ff43e68b7af31..1c182f13680179282f11f597efc5a37623ae9386 100644 (file)
--- 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 845486cf9216ae990b083606061c2c25a8682909..5316a872b6a8129bc8c4780cab89495345e967a1 100644 (file)
--- 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);
 
 }