From 0b8109e0b12eeeb20abdac2afe3e53e26bb164c6 Mon Sep 17 00:00:00 2001 From: prot0man Date: Mon, 15 Jan 2024 15:52:55 -0500 Subject: [PATCH 1/2] Added test for delete from in-memory zip. This required making some updates that are kind of hacky. The path of least edits to miniz necessitated updating miniz mz_zip_reader_init_mem to initialize the writer function and additionally zip_stream_open to handle the case where a user passes a stream and the mode 'r' or 'd'. --- src/miniz.h | 12 +++++++ src/zip.c | 9 ++++- test/test_entry.c | 85 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 105 insertions(+), 1 deletion(-) diff --git a/src/miniz.h b/src/miniz.h index cd864831..42a42760 100644 --- a/src/miniz.h +++ b/src/miniz.h @@ -5278,6 +5278,9 @@ mz_zip_array_range_check(const mz_zip_array *pArray, mz_uint index) { ((element_type *)((array_ptr)->m_p))[index] #endif +static size_t mz_zip_heap_write_func(void *pOpaque, mz_uint64 file_ofs, + const void *pBuf, size_t n); + static MZ_FORCEINLINE void mz_zip_array_init(mz_zip_array *pArray, mz_uint32 element_size) { memset(pArray, 0, sizeof(mz_zip_array)); @@ -5994,6 +5997,14 @@ static size_t mz_zip_mem_read_func(void *pOpaque, mz_uint64 file_ofs, return s; } +// Intentionally wrap the mz_zip_heap_write_func logic into a different function since +// there is logic that frees the original buffer during stream close if the writer +// function is equal to mz_zip_heap_write_func. +static size_t mz_zip_mem_write_func(void *pOpaque, mz_uint64 file_ofs, + const void *pBuf, size_t n) { + return mz_zip_heap_write_func(pOpaque, file_ofs, pBuf, n); +} + mz_bool mz_zip_reader_init_mem(mz_zip_archive *pZip, const void *pMem, size_t size, mz_uint flags) { if (!pMem) @@ -6008,6 +6019,7 @@ mz_bool mz_zip_reader_init_mem(mz_zip_archive *pZip, const void *pMem, pZip->m_zip_type = MZ_ZIP_TYPE_MEMORY; pZip->m_archive_size = size; pZip->m_pRead = mz_zip_mem_read_func; + pZip->m_pWrite = mz_zip_mem_write_func; pZip->m_pIO_opaque = pZip; pZip->m_pNeeds_keepalive = NULL; diff --git a/src/zip.c b/src/zip.c index 2480b71d..98f4e40f 100644 --- a/src/zip.c +++ b/src/zip.c @@ -594,6 +594,10 @@ static int zip_entry_finalize(struct zip_t *zip, return ZIP_EOOMEM; } + if(n == 0) { + return 0; + } + for (i = 0; i < n; ++i) { local_header_ofs_array[i] = entry_mark[i].m_local_header_ofs; ssize_t index = zip_sort(local_header_ofs_array, i); @@ -609,6 +613,7 @@ static int zip_entry_finalize(struct zip_t *zip, CLEANUP(local_header_ofs_array); return ZIP_EOOMEM; } + for (i = 0; i < n - 1; i++) { length[i] = (size_t)(local_header_ofs_array[i + 1] - local_header_ofs_array[i]); @@ -1835,7 +1840,9 @@ struct zip_t *zip_stream_openwitherror(const char *stream, size_t size, } zip->level = (mz_uint)level; - if ((stream != NULL) && (size > 0) && (mode == 'r')) { + // for modes 'd' and 'w', would be better to use mz_zip_reader_init_writer, but there's no clean + // way to load the existing stream with that. + if ((stream != NULL) && (size > 0) && (mode == 'r' || mode == 'd' || mode == 'w')) { if (!mz_zip_reader_init_mem(&(zip->archive), stream, size, 0)) { *errnum = ZIP_ERINIT; goto cleanup; diff --git a/test/test_entry.c b/test/test_entry.c index a0c40256..7fae49d8 100644 --- a/test/test_entry.c +++ b/test/test_entry.c @@ -387,6 +387,90 @@ MU_TEST(test_entries_delete) { zip_close(zip); } +MU_TEST(test_entries_delete_stream) { + char *entries[] = {"delete.me", "_", "delete/file.1", "deleteme/file.3", + "delete/file.2"}; + FILE *fh = NULL; + size_t zsize = 0; + uint8_t *zdata = NULL, *modified_zdata = NULL; + int rc = 0; + + // read zip into in memory buffer + fh = fopen(ZIPNAME, "rb"); + mu_check(fh != NULL); + + rc = fseek(fh, 0L, SEEK_END); + mu_check(rc != -1); + + zsize = ftell(fh); + + rc = fseek(fh, 0L, SEEK_SET); + mu_check(rc != -1); + + zdata = (uint8_t *)malloc(zsize); + mu_check(zdata != NULL); + + rc = fread(zdata, zsize, 1, fh); + mu_check(rc >= 1); + + fclose(fh); + fh = NULL; + + struct zip_t *zip = zip_stream_open(zdata, zsize, 0, 'd'); + mu_check(zip != NULL); + + mu_assert_int_eq(5, zip_entries_delete(zip, entries, 5)); + + zip_stream_copy(zip, (void **)&modified_zdata, &zsize); + mu_check(modified_zdata != NULL); + + zip_stream_close(zip); + free(zdata); + zdata = NULL; + + zip = zip_stream_open(modified_zdata, zsize, 0, 'r'); + mu_check(zip != NULL); + + mu_assert_int_eq(ZIP_ENOENT, zip_entry_open(zip, "delete.me")); + mu_assert_int_eq(0, zip_entry_close(zip)); + fprintf(stdout, "delete.me: %s\n", zip_strerror(ZIP_ENOENT)); + + mu_assert_int_eq(ZIP_ENOENT, zip_entry_open(zip, "_")); + mu_assert_int_eq(0, zip_entry_close(zip)); + fprintf(stdout, "_: %s\n", zip_strerror(ZIP_ENOENT)); + + mu_assert_int_eq(ZIP_ENOENT, zip_entry_open(zip, "delete/file.1")); + mu_assert_int_eq(0, zip_entry_close(zip)); + fprintf(stdout, "delete/file.1: %s\n", zip_strerror(ZIP_ENOENT)); + + mu_assert_int_eq(ZIP_ENOENT, zip_entry_open(zip, "deleteme/file.3")); + mu_assert_int_eq(0, zip_entry_close(zip)); + fprintf(stdout, "delete/file.3: %s\n", zip_strerror(ZIP_ENOENT)); + + mu_assert_int_eq(ZIP_ENOENT, zip_entry_open(zip, "delete/file.2")); + mu_assert_int_eq(0, zip_entry_close(zip)); + fprintf(stdout, "delete/file.2: %s\n", zip_strerror(ZIP_ENOENT)); + + mu_assert_int_eq(total_entries - 5, zip_entries_total(zip)); + + mu_assert_int_eq(0, zip_entry_open(zip, "delete/file.4")); + + size_t buftmp = 0; + char *buf = NULL; + ssize_t bufsize = zip_entry_read(zip, (void **)&buf, &buftmp); + + mu_assert_int_eq(bufsize, strlen(TESTDATA2)); + mu_assert_int_eq((size_t)bufsize, buftmp); + mu_assert_int_eq(0, strncmp(buf, TESTDATA2, bufsize)); + mu_assert_int_eq(0, zip_entry_close(zip)); + + free(buf); + buf = NULL; + + zip_stream_close(zip); + free(modified_zdata); +} + MU_TEST(test_entry_offset) { struct zip_t *zip = zip_open(ZIPNAME, 0, 'r'); mu_check(zip != NULL); @@ -423,6 +507,7 @@ MU_TEST_SUITE(test_entry_suite) { MU_RUN_TEST(test_list_entries); MU_RUN_TEST(test_entries_deletebyindex); MU_RUN_TEST(test_entries_delete); + MU_RUN_TEST(test_entries_delete_stream); MU_RUN_TEST(test_entry_offset); } From 7c1646b7933705c6c7884b83c9eb9d386e3be285 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kuba=20Podg=C3=B3rski?= Date: Mon, 15 Jan 2024 23:44:34 +0100 Subject: [PATCH 2/2] Upgrade cmake to 3.17 --- .github/workflows/build.yml | 14 ++++++++++---- CMakeLists.txt | 2 +- src/zip.c | 19 ++++++++++++++----- test/CMakeLists.txt | 2 +- test/test_entry.c | 7 +++++-- 5 files changed, 31 insertions(+), 13 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 795eafe3..4a64ff77 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -17,7 +17,9 @@ jobs: cmake --build build tree -sha build - name: Test - run: ASAN_OPTIONS=detect_leaks=0 LSAN_OPTIONS=verbosity=1:log_threads=1 cmake --build build --target test + run: | + cd build + ASAN_OPTIONS=detect_leaks=0 LSAN_OPTIONS=verbosity=1:log_threads=1 CTEST_OUTPUT_ON_FAILURE=1 ctest -VV --output-on-failure macos: runs-on: macos-latest @@ -33,7 +35,9 @@ jobs: cmake --build build tree -sha build - name: Test - run: cmake --build build --target test + run: | + cd build + CTEST_OUTPUT_ON_FAILURE=1 ctest -VV --output-on-failure # freebsd: # runs-on: macos-latest @@ -63,7 +67,7 @@ jobs: - name: Test run: | cd build - ctest -VV -C "Debug" + ctest -VV -C "Debug" --output-on-failure windows-mingw: runs-on: "windows-latest" @@ -76,4 +80,6 @@ jobs: cmake --build build tree /a /f build - name: Test - run: cmake --build build --target test + run: | + cd build + ctest -VV -C "Debug" --output-on-failure diff --git a/CMakeLists.txt b/CMakeLists.txt index 8f10e0c1..685f6f93 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,4 +1,4 @@ -cmake_minimum_required(VERSION 3.14) +cmake_minimum_required(VERSION 3.17) project(zip LANGUAGES C diff --git a/src/zip.c b/src/zip.c index 98f4e40f..2c24e9e2 100644 --- a/src/zip.c +++ b/src/zip.c @@ -589,15 +589,16 @@ static int zip_entry_finalize(struct zip_t *zip, const ssize_t n) { ssize_t i = 0; - mz_uint64 *local_header_ofs_array = (mz_uint64 *)calloc(n, sizeof(mz_uint64)); - if (!local_header_ofs_array) { - return ZIP_EOOMEM; - } if(n == 0) { return 0; } + mz_uint64 *local_header_ofs_array = (mz_uint64 *)calloc(n, sizeof(mz_uint64)); + if (!local_header_ofs_array) { + return ZIP_EOOMEM; + } + for (i = 0; i < n; ++i) { local_header_ofs_array[i] = entry_mark[i].m_local_header_ofs; ssize_t index = zip_sort(local_header_ofs_array, i); @@ -1843,7 +1844,15 @@ struct zip_t *zip_stream_openwitherror(const char *stream, size_t size, // for modes 'd' and 'w', would be better to use mz_zip_reader_init_writer, but there's no clean // way to load the existing stream with that. if ((stream != NULL) && (size > 0) && (mode == 'r' || mode == 'd' || mode == 'w')) { - if (!mz_zip_reader_init_mem(&(zip->archive), stream, size, 0)) { + uint8_t *stream_copy = (uint8_t *)malloc(size); + + if(!stream_copy) { + *errnum = ZIP_EOOMEM; + goto cleanup; + } + memcpy(stream_copy, stream, size); + + if (!mz_zip_reader_init_mem(&(zip->archive), stream_copy, size, 0)) { *errnum = ZIP_ERINIT; goto cleanup; } diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 3a8d7cab..5dae8aa6 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -1,4 +1,4 @@ -cmake_minimum_required(VERSION 3.14) +cmake_minimum_required(VERSION 3.17) find_package(Sanitizers) diff --git a/test/test_entry.c b/test/test_entry.c index 7fae49d8..857edffc 100644 --- a/test/test_entry.c +++ b/test/test_entry.c @@ -410,7 +410,7 @@ MU_TEST(test_entries_delete_stream) { zdata = (uint8_t *)malloc(zsize); mu_check(zdata != NULL); - rc = fread(zdata, zsize, 1, fh); + rc = fread(zdata, sizeof(uint8_t), zsize, fh); mu_check(rc >= 1); fclose(fh); @@ -424,10 +424,13 @@ MU_TEST(test_entries_delete_stream) { zip_stream_copy(zip, (void **)&modified_zdata, &zsize); mu_check(modified_zdata != NULL); - zip_stream_close(zip); free(zdata); zdata = NULL; + // Note that zip_stream_close will free the zdata passed in zip_stream_open + zip_stream_close(zip); + zdata = NULL; + zip = zip_stream_open(modified_zdata, zsize, 0, 'r'); mu_check(zip != NULL);