From b47f61bc252a84450fe31c24ce031a1509bf4373 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20K=C3=B6lling?= Date: Mon, 23 Aug 2021 17:48:39 +0200 Subject: [PATCH 1/3] added test for opening a trunceded file using in-memory open This test currently fails. When trying to open an in-memory file which is only available partially (i.e. by accidental truncation), then the library should fail with an error in stead of an assertation, such that user code can react on this. --- nc_test4/CMakeLists.txt | 2 +- nc_test4/tst_broken_files.c | 41 +++++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 nc_test4/tst_broken_files.c diff --git a/nc_test4/CMakeLists.txt b/nc_test4/CMakeLists.txt index c5c345005b..780aea11e4 100644 --- a/nc_test4/CMakeLists.txt +++ b/nc_test4/CMakeLists.txt @@ -20,7 +20,7 @@ SET(NC4_TESTS tst_dims tst_dims2 tst_dims3 tst_files tst_files4 tst_files6 tst_sync tst_h_strbug tst_h_refs tst_h_scalar tst_rename tst_rename2 tst_rename3 tst_h5_endians tst_atts_string_rewrite tst_put_vars_two_unlim_dim tst_hdf5_file_compat tst_fill_attr_vanish tst_rehash tst_types tst_bug324 - tst_atts3 tst_put_vars tst_elatefill tst_udf tst_bug1442) + tst_atts3 tst_put_vars tst_elatefill tst_udf tst_bug1442 tst_broken_files) # Note, renamegroup needs to be compiled before run_grp_rename diff --git a/nc_test4/tst_broken_files.c b/nc_test4/tst_broken_files.c new file mode 100644 index 0000000000..af68b26905 --- /dev/null +++ b/nc_test4/tst_broken_files.c @@ -0,0 +1,41 @@ +/* This is part of the netCDF package. + Copyright 2018 University Corporation for Atmospheric Research/Unidata + See COPYRIGHT file for conditions of use. + + Test that netCDF provides proper error messages if broken Files are supplied. +*/ + +#include +#include +#include +#include "err_macros.h" +#include "netcdf.h" +#include "netcdf_mem.h" + +#include + +#define FILE_NAME "tst_broken_files.nc" +#define TRUNCATED_FILE_CONTENT "\x89HDF\r\n\x1a\n" + +int +main() { + printf("\n*** Testing NetCDF-4 with truncated (broken) sample file.\n"); + { + printf("*** testing via file on file-system ...\n"); + FILE *fp = fopen(FILE_NAME, "w"); + if(!fp) ERR; + if(fwrite(TRUNCATED_FILE_CONTENT, sizeof(char), sizeof(TRUNCATED_FILE_CONTENT), fp) != sizeof(TRUNCATED_FILE_CONTENT)) ERR; + fclose(fp); + + int ncid; + if (nc_open(FILE_NAME, 0, &ncid) != NC_EHDFERR) ERR; + } + + { + printf("*** testing via in-memory access ...\n"); + int ncid; + if (nc_open_mem(FILE_NAME, 0, sizeof(TRUNCATED_FILE_CONTENT), TRUNCATED_FILE_CONTENT, &ncid) != NC_EHDFERR) ERR; + } + SUMMARIZE_ERR; + FINAL_RESULTS; +} From b58a3ff07a826b4e8903ecd40f0fa328a6d4a4f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20K=C3=B6lling?= Date: Tue, 24 Aug 2021 17:11:05 +0200 Subject: [PATCH 2/3] allow missing udata when closing file with abort=1 If a memory backed file is closed due to an aborted opening (i.e. a broken file was attempted to be opened), udata may not be set. In this case, the assertation checking for udata should not be triggered. --- libhdf5/hdf5file.c | 4 ++-- libhdf5/nc4memcb.c | 21 ++++++++++++--------- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/libhdf5/hdf5file.c b/libhdf5/hdf5file.c index bdb4ddcb4b..4945c87326 100644 --- a/libhdf5/hdf5file.c +++ b/libhdf5/hdf5file.c @@ -16,7 +16,7 @@ #include "ncrc.h" #include "ncauth.h" -extern int NC4_extract_file_image(NC_FILE_INFO_T* h5); /* In nc4memcb.c */ +extern int NC4_extract_file_image(NC_FILE_INFO_T* h5, int abort); /* In nc4memcb.c */ static void dumpopenobjects(NC_FILE_INFO_T* h5); @@ -242,7 +242,7 @@ nc4_close_netcdf4_file(NC_FILE_INFO_T *h5, int abort, NC_memio *memio) if (h5->mem.inmemory) { /* Pull out the final memory */ - (void)NC4_extract_file_image(h5); + (void)NC4_extract_file_image(h5, abort); if (!abort && memio != NULL) { *memio = h5->mem.memio; /* capture it */ diff --git a/libhdf5/nc4memcb.c b/libhdf5/nc4memcb.c index 3228d2bceb..4aa42bdb2a 100644 --- a/libhdf5/nc4memcb.c +++ b/libhdf5/nc4memcb.c @@ -854,22 +854,25 @@ NC4_image_finalize(void* _udata) } int -NC4_extract_file_image(NC_FILE_INFO_T* h5) +NC4_extract_file_image(NC_FILE_INFO_T* h5, int abort) { int stat = NC_NOERR; H5LT_file_image_ud_t *udata; udata = (H5LT_file_image_ud_t *)h5->mem.udata; - assert(udata != NULL); + if(abort && udata == NULL) { + stat = NC_EHDFERR; + } else { + assert(udata != NULL); - /* Fill in h5->mem.memio from udata */ - h5->mem.memio.memory = udata->vfd_image_ptr; - h5->mem.memio.size = udata->vfd_image_size; + /* Fill in h5->mem.memio from udata */ + h5->mem.memio.memory = udata->vfd_image_ptr; + h5->mem.memio.size = udata->vfd_image_size; - /* Move control */ - udata->vfd_image_ptr = NULL; - udata->vfd_image_size = 0; - + /* Move control */ + udata->vfd_image_ptr = NULL; + udata->vfd_image_size = 0; + } return stat; } From 3eaecb41b7399d72d4f20c5b2252908552e109f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20K=C3=B6lling?= Date: Thu, 26 Aug 2021 16:12:24 +0200 Subject: [PATCH 3/3] hand over udata to h5->mem a little earlier I think udata will be freed by the calback as soon as the callbacks are set and H5Pclose is called, so the backward link should be set up at that place. Without this change, local_image_free may be called without a proper reference to udata, which may raise an assertion: assert(udata->fapl_image_ptr == udata->app_image_ptr); while trying to open a broken file. --- libhdf5/nc4memcb.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/libhdf5/nc4memcb.c b/libhdf5/nc4memcb.c index 4aa42bdb2a..0087f36033 100644 --- a/libhdf5/nc4memcb.c +++ b/libhdf5/nc4memcb.c @@ -794,6 +794,10 @@ NC4_image_init(NC_FILE_INFO_T* h5) if (H5Pset_file_image(fapl, udata->app_image_ptr, udata->app_image_size) < 0) goto out; + /* Maintain a backward link */ + h5->mem.udata = (void*)udata; + udata = NULL; + /* define a unique file name */ snprintf(file_name, (sizeof(file_name) - 1), "file_image_%ld", file_name_counter++); @@ -814,10 +818,6 @@ NC4_image_init(NC_FILE_INFO_T* h5) goto out; } - /* Maintain a backward link */ - h5->mem.udata = (void*)udata; - udata = NULL; - done: /* Reclaim the fapl object */ H5E_BEGIN_TRY {