Skip to content

Commit

Permalink
Fix handling of '/' characters in names in DAP2.
Browse files Browse the repository at this point in the history
re: Unidata/thredds#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.
  • Loading branch information
DennisHeimbigner committed Feb 15, 2019
1 parent 30c2ca2 commit c59d5ce
Show file tree
Hide file tree
Showing 5 changed files with 9 additions and 30 deletions.
7 changes: 0 additions & 7 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
3 changes: 0 additions & 3 deletions config.h.cmake.in
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
15 changes: 0 additions & 15 deletions configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand Down
4 changes: 4 additions & 0 deletions libdap2/daputil.c
Original file line number Diff line number Diff line change
Expand Up @@ -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,"/");
}

Expand Down Expand Up @@ -733,13 +735,15 @@ dap_badname(char* name)
return 0;
}

#if 0
/* Repair a dap name */
char*
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
Expand Down
10 changes: 5 additions & 5 deletions libdap4/d4varx.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;}
}
}

Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit c59d5ce

Please sign in to comment.