From c59d5ce205a58a2832cffac49b54ce82a659e20d Mon Sep 17 00:00:00 2001 From: Dennis Heimbigner Date: Thu, 14 Feb 2019 20:25:40 -0700 Subject: [PATCH] Fix handling of '/' characters in names in DAP2. re: https://github.com/Unidata/thredds/issues/1224 [note that this is an issue in thredds, but the fix is in netcdf-c] A thredds server can encode a netcdf-4 file into DAP2 by flattening names to include the containing group path, where the group names are separated by '/'. But the '/' is prohibited in netcdf names even if escaped (a decision before my time). So, if the netcdf-c/libdap2 code encounters a DAP2 name with '/' characters, the '/' characters are converted to the string %2f. Unfortunately, there is a glitch, namely that converting the leading '/' produces a name that is still illegal. This PR modifies the code to just drop the leading '/' character. --- CMakeLists.txt | 7 ------- config.h.cmake.in | 3 --- configure.ac | 15 --------------- libdap2/daputil.c | 4 ++++ libdap4/d4varx.c | 10 +++++----- 5 files changed, 9 insertions(+), 30 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 0a77fde273..c998aa54f7 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -846,13 +846,6 @@ OPTION(ENABLE_DAP_LONG_TESTS "Enable DAP long tests." OFF) OPTION(ENABLE_DAP_REMOTE_TESTS "Enable DAP remote tests." ON) SET(REMOTETESTSERVERS "149.165.169.123:8080,remotetest.unidata.ucar.edu" CACHE STRING "test servers to use for remote test") -# If netCDF4 and DAP, Option for DAP groups. -IF(ENABLE_NETCDF_4 AND ENABLE_DAP2) - OPTION(ENABLE_DAP_GROUPS "Whether netcdf4 group names should be enabled." ON) -ELSE() - SET(ENABLE_DAP_GROUPS OFF CACHE BOOL "Whether netcdf4 group names should be enabled.") -ENDIF() - # Enable some developer-only tests OPTION(ENABLE_EXTRA_TESTS "Enable Extra tests. Some may not work because of known issues. Developers only." OFF) IF(ENABLE_EXTRA_TESTS) diff --git a/config.h.cmake.in b/config.h.cmake.in index a806a9b000..9c145fb135 100644 --- a/config.h.cmake.in +++ b/config.h.cmake.in @@ -132,9 +132,6 @@ are set when opening a binary file on Windows. */ /* if true, build DAP4 Client */ #cmakedefine ENABLE_DAP4 1 -/* if true, enable DAP group names */ -#cmakedefine ENABLE_DAP_GROUPS 1 - /* if true, do remote tests */ #cmakedefine ENABLE_DAP_REMOTE_TESTS 1 diff --git a/configure.ac b/configure.ac index e853aa2557..828d5501fd 100644 --- a/configure.ac +++ b/configure.ac @@ -395,21 +395,6 @@ if test "x$enable_dap_remote_tests" = "xno" ; then fi AC_MSG_RESULT($enable_dap_auth_tests) -# Control if groups are supported in [netcdf4]dap2 code -AC_MSG_CHECKING([whether [netcdf4] group names for DAP2 should be enabled (default on)]) -AC_ARG_ENABLE([dap-groups], - [AS_HELP_STRING([--disable-dap-groups], - [disable [netcdf4] DAP2 group names])]) -test "x$enable_dap_groups" = xno || enable_dap_groups=yes -AC_MSG_RESULT($enable_dap_groups) -if test "x$enable_dap" = "xno" ; then - AC_MSG_NOTICE([DAP2 groups is being disabled because DAP2 support is disabled or netcdf-4 disabled]) - enable_dap_groups=no -fi -if test "x$enable_dap_groups" = xyes; then -AC_DEFINE([ENABLE_DAP_GROUPS], [1], [if true, enable DAP group names]) -fi - # Did the user specify a list of test servers to try for remote tests? AC_MSG_CHECKING([which remote test server(s) to use]) AC_ARG_WITH([testservers], diff --git a/libdap2/daputil.c b/libdap2/daputil.c index d2348c0e0e..8c10a54300 100644 --- a/libdap2/daputil.c +++ b/libdap2/daputil.c @@ -34,6 +34,8 @@ names to %2f. char* cdflegalname(char* name) { + if(name != NULL && name[0] == '/') + name = name+1; /* remove leading / so name will be legal */ return repairname(name,"/"); } @@ -733,6 +735,7 @@ dap_badname(char* name) return 0; } +#if 0 /* Repair a dap name */ char* dap_repairname(char* name) @@ -740,6 +743,7 @@ dap_repairname(char* name) /* assume that dap_badname was called on this name and returned 1 */ return repairname(name,baddapchars); } +#endif /* Check a name to see if it contains illegal dap characters and repair them diff --git a/libdap4/d4varx.c b/libdap4/d4varx.c index ee7095ed3f..647afe213a 100644 --- a/libdap4/d4varx.c +++ b/libdap4/d4varx.c @@ -46,7 +46,7 @@ NCD4_get_vars(int ncid, int varid, size_t dstcount; if((ret=getvarx(ncid, varid, &info, &ncvar, &xtype, &xsize, &nc4type, &nc4size))) - {THROW(ret); goto done;} + {goto done;} meta = info->substrate.metadata; nctype = ncvar->basetype; @@ -91,17 +91,17 @@ NCD4_get_vars(int ncid, int varid, offset = ncvar->data.dap4data.memory; /* We have to walk to the count'th location in the data */ if((ret=NCD4_moveto(meta,ncvar,count,&offset))) - {THROW(ret); goto done;} + {goto done;} } dst = instance; if((ret=NCD4_fillinstance(meta,nctype,&offset,&dst,blobs))) - {THROW(ret); goto done;} + {goto done;} if(xtype == nc4type) { /* We can just copy out the data */ memcpy(xpos,instance,nc4size); } else { /* Need to convert */ if((ret=NCD4_convert(nc4type,xtype,xpos,instance,1))) - {THROW(ret); goto done;} + {goto done;} } } @@ -136,7 +136,7 @@ getvarx(int ncid, int varid, NCD4INFO** infop, NCD4node** varp, int grp_id; if((ret = NC_check_id(ncid, (NC**)&ncp)) != NC_NOERR) - {THROW(ret); goto done;} + goto done; info = getdap(ncp); if(info == NULL)