diff --git a/CMakeLists.txt b/CMakeLists.txt index 2e3235556..c1b89a43d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -501,6 +501,14 @@ IF(ENABLE_NETCDF_4) SET(ENABLE_NETCDF4 ON CACHE BOOL "") ENDIF() +# Option to allow for strict null file padding. +# See https://github.com/Unidata/netcdf-c/issues/657 for more information +OPTION(ENABLE_STRICT_NULL_BYTE_HEADER_PADDING "Enable strict null byte header padding." OFF) + +IF(ENABLE_STRICT_NULL_BYTE_HEADER_PADDING) + SET(USE_STRICT_NULL_BYTE_HEADER_PADDING ON CACHE BOOL "") +ENDIF(ENABLE_STRICT_NULL_BYTE_HEADER_PADDING) + # Option for building RPC OPTION(ENABLE_RPC "Enable RPC Client and Server." OFF) IF(ENABLE_RPC) @@ -826,7 +834,6 @@ ELSE() SET(ENABLE_DAP4 OFF) ENDIF() -# Check to see if libtool supports # Check for the math library so it can be explicitly linked. IF(NOT WIN32) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 0819f71af..450c31644 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -11,6 +11,7 @@ reading and writing. See the file docs/filters.md. ## 4.5.1 - TBD +* [Enhancement] Added an option to enable strict null-byte padding for headers; this padding was specified in the spec but was not enforced. Enabling this option will allow you to check your files, as it will return an E_NULLPAD error. It is possible for these files to have been written by older versions of libnetcdf. There is no effective problem caused by this lack of null padding, so enabling these options is informational only. The options for `configure` and `cmake` are `--enable-strict-null-byte-header-padding` and `-DENABLE_STRICT_NULL_BYTE_HEADER_PADDING`, respectively. See [Github #657](https://github.com/Unidata/netcdf-c/issues/657) for more information. * [Enhancement] Reverted behavior/handling of out-of-range attribute values to pre-4.5.0 default. See [Github #512](https://github.com/Unidata/netcdf-c/issues/512) for more information. * [Bug] Fixed error in tst_parallel2.c. See [Github #545](https://github.com/Unidata/netcdf-c/issues/545) for more information. * [Bug] Fixed handling of corrupt files + proper offset handling for hdf5 files. See [Github #552](https://github.com/Unidata/netcdf-c/issues/552) for more information. diff --git a/config.h.cmake.in b/config.h.cmake.in index 2c8228748..c06912525 100644 --- a/config.h.cmake.in +++ b/config.h.cmake.in @@ -111,6 +111,9 @@ are set when opening a binary file on Windows. */ #cmakedefine ENABLE_CDF5 1 #cmakedefine USE_CDF5 1 +/* if true, enable strict null byte header padding. */ +#cmakedefine USE_STRICT_NULL_BYTE_HEADER_PADDING 1 + /* if true, build DAP2 and DAP4 Client */ #cmakedefine ENABLE_DAP 1 diff --git a/configure.ac b/configure.ac index a76a00e1f..16fdb7786 100644 --- a/configure.ac +++ b/configure.ac @@ -442,6 +442,21 @@ AC_MSG_RESULT([$enable_dap_long_tests]) AM_CONDITIONAL(INTERNAL_OCLIB,[test "x" = "x"]) +# Check whether we want to enable strict null byte header padding. +# See https://github.com/Unidata/netcdf-c/issues/657 for more information. +AC_MSG_CHECKING([whether to enable strict null-byte header padding when reading (default off)]) +AC_ARG_ENABLE([strict-null-byte-header-padding], + [AS_HELP_STRING([--enable-strict-null-byte-header-padding], + [enable strict null-byte header padding when reading netCDF3 files.])]) +test "x$enable_strict_null_byte_header_padding" = xyes || enable_strict_null_byte_header_padding=no +AC_MSG_RESULT($enable_strict_null_byte_header_padding) + +if test "x$enable_strict_null_byte_header_padding" = xyes; then + AC_DEFINE([USE_STRICT_NULL_BYTE_HEADER_PADDING], [1], [if true, enable strict null byte header padding]) +fi + +AM_CONDITIONAL(USE_STRICT_NULL_BYTE_HEADER_PADDING, [test x$enable_strict_null_byte_header_padding = xyes ]) + # Check whether we want to enable CDF5 support. AC_MSG_CHECKING([whether CDF5 support should be enabled (default off)]) AC_ARG_ENABLE([cdf5], @@ -1098,7 +1113,7 @@ if test "x$enable_pnetcdf" = xyes; then if test "x$pnetcdf_conflict" = xyes ; then AC_MSG_ERROR([Cannot link to pnetcdf library.]) - fi + fi # Pnetcdf did not support utf-8 until 1.6.0 @@ -1279,6 +1294,7 @@ AM_CONDITIONAL(USE_DAP, [test "x$enable_dap" = xyes]) # Alias # Provide protocol specific flags AM_CONDITIONAL(ENABLE_DAP, [test "x$enable_dap" = xyes]) AM_CONDITIONAL(ENABLE_DAP4, [test "x$enable_dap4" = xyes]) +AM_CONDITIONAL(USE_STRICT_NULL_BYTE_HEADER_PADDING, [test x$enable_strict_null_byte_header_padding = xyes]) AM_CONDITIONAL(ENABLE_CDF5, [test "x$enable_cdf5" = xyes]) AM_CONDITIONAL(ENABLE_DAP_REMOTE_TESTS, [test "x$enable_dap_remote_tests" = xyes]) AM_CONDITIONAL(ENABLE_DAP_AUTH_TESTS, [test "x$enable_dap_auth_tests" = xyes]) @@ -1545,4 +1561,3 @@ AC_OUTPUT() #mv -f ${abs_top_srcdir}/test_common.sh ${abs_top_builddir}/test_common.sh cat libnetcdf.settings - diff --git a/docs/windows-binaries.md b/docs/windows-binaries.md index 307c80c62..54e9172cd 100644 --- a/docs/windows-binaries.md +++ b/docs/windows-binaries.md @@ -36,7 +36,7 @@ The included dependencies and versions are as follows: Configuration | 32-bit | 64-bit | :-------------------|:-------- |:-------| -netCDF 3 | [netCDFmajor.minor.point-NC3-32.exe][r1] | [netCDFmajor.minor.point-NC3-64.exe][r6] +netCDF 3 | [netCDFmajor.minor.point-NC3-32.exe][r1] | [netCDFmajor.minor.point-NC3-64.exe][r5] netCDF3+DAP | [netCDFmajor.minor.point-NC3-DAP-32.exe][r2] | [netCDFmajor.minor.point-NC3-DAP-64.exe][r6] netCDF4 | [netCDFmajor.minor.point-NC4-32.exe][r3] | [netCDFmajor.minor.point-NC4-64.exe][r7] netCDF4+DAP | [netCDFmajor.minor.point-NC4-DAP-32.exe][r4] | [netCDFmajor.minor.point-NC4-DAP-64.exe][r8] @@ -64,7 +64,7 @@ When installed, the netCDF libraries are placed in the specified locations, alon [r2]: http://www.unidata.ucar.edu/downloads/netcdf/ftp/netCDFmajor.minor.point-NC3-DAP-32.exe [r3]: http://www.unidata.ucar.edu/downloads/netcdf/ftp/netCDFmajor.minor.point-NC4-32.exe [r4]: http://www.unidata.ucar.edu/downloads/netcdf/ftp/netCDFmajor.minor.point-NC4-DAP-32.exe -[r6]: http://www.unidata.ucar.edu/downloads/netcdf/ftp/netCDFmajor.minor.point-NC3-64.exe +[r5]: http://www.unidata.ucar.edu/downloads/netcdf/ftp/netCDFmajor.minor.point-NC3-64.exe [r6]: http://www.unidata.ucar.edu/downloads/netcdf/ftp/netCDFmajor.minor.point-NC3-DAP-64.exe [r7]: http://www.unidata.ucar.edu/downloads/netcdf/ftp/netCDFmajor.minor.point-NC4-64.exe [r8]: http://www.unidata.ucar.edu/downloads/netcdf/ftp/netCDFmajor.minor.point-NC4-DAP-64.exe diff --git a/include/netcdf.h b/include/netcdf.h index 421fc3407..4b131fa43 100644 --- a/include/netcdf.h +++ b/include/netcdf.h @@ -450,7 +450,8 @@ by the desired type. */ #define NC_EFILTER (-132) /**< Filter operation failed. */ #define NC_ERCFILE (-133) /**< RC file failure */ -#define NC4_LAST_ERROR (-134) +#define NC_ENULLPAD (-134) /**< Header Bytes not Null-Byte padded */ +#define NC4_LAST_ERROR (-135) /* This is used in netCDF-4 files for dimensions without coordinate * vars. */ diff --git a/libdispatch/derror.c b/libdispatch/derror.c index c91eeb342..733fbf27e 100644 --- a/libdispatch/derror.c +++ b/libdispatch/derror.c @@ -259,6 +259,8 @@ const char *nc_strerror(int ncerr1) case NC_EMPI: return "NetCDF: MPI operation failed."; case NC_ERCFILE: return "NetCDF: RC File Failure."; + case NC_ENULLPAD: + return "NetCDF: File fails strict Null-Byte Header check."; default: #ifdef USE_PNETCDF /* The behavior of ncmpi_strerror here is to return diff --git a/libsrc/v1hpg.c b/libsrc/v1hpg.c index 07c322ce5..0e834a71d 100644 --- a/libsrc/v1hpg.c +++ b/libsrc/v1hpg.c @@ -303,9 +303,9 @@ v1h_put_NC_string(v1hs *psp, const NC_string *ncstrp) static int v1h_get_NC_string(v1hs *gsp, NC_string **ncstrpp) { - int status; - size_t padding, nchars = 0; - NC_string *ncstrp; + int status = 0; + size_t padding = 0, nchars = 0; + NC_string *ncstrp = NULL; status = v1h_get_size_t(gsp, &nchars); if(status != NC_NOERR) @@ -335,15 +335,18 @@ v1h_get_NC_string(v1hs *gsp, NC_string **ncstrpp) padding = _RNDUP(X_SIZEOF_CHAR * ncstrp->nchars, X_ALIGN) - X_SIZEOF_CHAR * ncstrp->nchars; + +#if USE_STRICT_NULL_BYTE_HEADER_PADDING if (padding > 0) { /* CDF specification: Header padding uses null (\x00) bytes. */ char pad[X_ALIGN-1]; memset(pad, 0, X_ALIGN-1); if (memcmp((char*)gsp->pos-padding, pad, padding) != 0) { free_NC_string(ncstrp); - return NC_EINVAL; + return NC_ENULLPAD; } } +#endif *ncstrpp = ncstrp; @@ -618,11 +621,11 @@ ncmpix_len_nctype(nc_type type) { static int v1h_put_NC_attrV(v1hs *psp, const NC_attr *attrp) { - int status; + int status = 0; const size_t perchunk = psp->extent; size_t remaining = attrp->xsz; void *value = attrp->xvalue; - size_t nbytes, padding; + size_t nbytes = 0, padding = 0; assert(psp->extent % X_ALIGN == 0); @@ -641,6 +644,7 @@ v1h_put_NC_attrV(v1hs *psp, const NC_attr *attrp) } while(remaining != 0); + padding = attrp->xsz - ncmpix_len_nctype(attrp->type) * attrp->nelems; if (padding > 0) { /* CDF specification: Header padding uses null (\x00) bytes. */ @@ -707,13 +711,16 @@ v1h_get_NC_attrV(v1hs *gsp, NC_attr *attrp) } while(remaining != 0); padding = attrp->xsz - ncmpix_len_nctype(attrp->type) * attrp->nelems; + +#if USE_STRICT_NULL_BYTE_HEADER_PADDING if (padding > 0) { /* CDF specification: Header padding uses null (\x00) bytes. */ char pad[X_ALIGN-1]; memset(pad, 0, X_ALIGN-1); if (memcmp((char*)gsp->pos-padding, pad, (size_t)padding) != 0) - return NC_EINVAL; + return NC_ENULLPAD; } +#endif return NC_NOERR; } diff --git a/nc_test/util.c b/nc_test/util.c index dcbc6f61e..d3cb4f0d8 100644 --- a/nc_test/util.c +++ b/nc_test/util.c @@ -1319,7 +1319,8 @@ char* nc_err_code_name(int err) case (NC_EDISKLESS): return "NC_EDISKLESS"; case (NC_ECANTEXTEND): return "NC_ECANTEXTEND"; case (NC_EMPI): return "NC_EMPI"; - // case (NC_EURL): return "NC_EURL"; + case (NC_ENULLPAD): return "NC_NULLPAD"; + // case (NC_EURL): return "NC_EURL"; // case (NC_ECONSTRAINT): return "NC_ECONSTRAINT"; #ifdef USE_PNETCDF case (NC_ESMALL): return "NC_ESMALL"; diff --git a/ncdump/CMakeLists.txt b/ncdump/CMakeLists.txt index e8844269f..f2513a808 100644 --- a/ncdump/CMakeLists.txt +++ b/ncdump/CMakeLists.txt @@ -116,6 +116,11 @@ ENDIF() add_sh_test(ncdump tst_hdf5_offset) ENDIF(USE_NETCDF4) + add_sh_test(ncdump tst_null_byte_padding) + IF(USE_STRICT_NULL_BYTE_HEADER_PADDING) + SET_TESTS_PROPERTIES(ncdump_tst_null_byte_padding PROPERTIES WILL_FAIL TRUE) + ENDIF(USE_STRICT_NULL_BYTE_HEADER_PADDING) + add_sh_test(ncdump tst_nccopy3) add_sh_test(ncdump tst_nccopy3_subset) add_sh_test(ncdump tst_charfill) diff --git a/ncdump/Makefile.am b/ncdump/Makefile.am index 4ba246426..c2d00043e 100644 --- a/ncdump/Makefile.am +++ b/ncdump/Makefile.am @@ -12,7 +12,7 @@ LDADD = ${top_builddir}/liblib/libnetcdf.la #TEST_LOG_DRIVER = $(SHELL) $(top_srcdir)/test-driver-verbose TESTS_ENVIRONMENT=CC=${CC} - +XFAIL_TESTS="" # This is the program we're building, and it's sources. bin_PROGRAMS = ncdump ncdump_SOURCES = ncdump.c vardata.c dumplib.c indent.c nctime0.c \ @@ -55,6 +55,11 @@ check_PROGRAMS += tst_fileinfo TESTS += tst_fileinfo.sh tst_hdf5_offset.sh run_ncgen_nc4_tests.sh endif +TESTS += tst_null_byte_padding.sh +if USE_STRICT_NULL_BYTE_HEADER_PADDING +XFAIL_TESTS += tst_null_byte_padding.sh +endif + if LARGE_FILE_TESTS TESTS += tst_iter.sh endif @@ -151,7 +156,8 @@ tst_fileinfo.sh run_ncgen_tests.sh test_360_day_1900.nc \ test_365_day_1900.nc test_366_day_1900.nc ref_test_360_day_1900.cdl \ ref_test_365_day_1900.cdl ref_test_366_day_1900.cdl \ tst_hdf5_offset.sh run_ncgen_nc4_tests.sh tst_nccopy3_subset.sh \ -ref_nccopy3_subset.nc test_corrupt_magic.nc +ref_nccopy3_subset.nc test_corrupt_magic.nc ref_null_byte_padding_test.nc \ +tst_null_byte_padding.sh # The L512.bin file is file containing exactly 512 bytes each of value 0. # It is used for creating hdf5 files with varying offsets for testing. diff --git a/ncdump/ref_null_byte_padding_test.nc b/ncdump/ref_null_byte_padding_test.nc new file mode 100644 index 000000000..40d7da128 Binary files /dev/null and b/ncdump/ref_null_byte_padding_test.nc differ diff --git a/ncdump/tst_null_byte_padding.sh b/ncdump/tst_null_byte_padding.sh new file mode 100755 index 000000000..50deb869e --- /dev/null +++ b/ncdump/tst_null_byte_padding.sh @@ -0,0 +1,17 @@ +#!/bin/sh + +if test "x$srcdir" = x ; then srcdir=`pwd`; fi +. ../test_common.sh + +# This shell script tests a file which doesn't adhere to the +# documented null-byte padding for header information. +# See https://github.com/Unidata/netcdf-c/issues/657 for more info. + +set -e + +echo "" +echo "*** Testing compatibility with non-null-byte padded test file." +${NCDUMP} $srcdir/ref_null_byte_padding_test.nc + +echo "Passed." +exit 0