Skip to content

Commit

Permalink
Make the dap4 code resistant to various server errors.
Browse files Browse the repository at this point in the history 
Some versions of some servers are returning malformed responses.
Make the library either handle them or gracefully fail.
The three server errors "fixed" here are as follows.
1. The attribute _NCProperties sometimes has a trailing nul character
   in its value. Soln is to elide the nul(s).
2. Sometimes a DAP response has no data part, only a DMR.
   Soln is to detect and return an error code instead of crashing.
3. Sometimes a server returns a redirection, but our current
   openmagic() function was not following the redirect. Soln
   is to follow redirects.
Also because of Unidata#2, I am temporarily making --disable-dap-remote-tests
be the default.
  • Loading branch information
@DennisHeimbigner
DennisHeimbigner committed Jan 8, 2020
1 parent d9eb078 commit f587654
Show file tree
Hide file tree
Showing 8 changed files with 77 additions and 39 deletions.
5 changes: 3 additions & 2 deletions configure.ac
View file
Original file line number Diff line number Diff line change
Expand Up @@ -365,11 +365,12 @@ fi
# --enable-dap => enable-dap4
enable_dap4=$enable_dap
# Default is to do the short remote tests.
# Temporary: Change default to npt do these tests
AC_MSG_CHECKING([whether dap remote testing should be enabled (default on)])
AC_ARG_ENABLE([dap-remote-tests],
[AS_HELP_STRING([--disable-dap-remote-tests],
[disable dap remote tests])])
test "x$enable_dap_remote_tests" = xno || enable_dap_remote_tests=yes
test "x$enable_dap_remote_tests" = xyes || enable_dap_remote_tests=no
if test "x$enable_dap" = "xno" ; then
enable_dap_remote_tests=no
fi
Expand Down Expand Up @@ -399,7 +400,7 @@ AC_ARG_WITH([testservers],
[REMOTETESTSERVERS=$with_testservers], [REMOTETESTSERVERS=no])
msg="$REMOTETESTSERVERS"
if test "x$REMOTETESTSERVERS" = xno ; then
svclist="149.165.169.123:8080,remotetest.unidata.ucar.edu"
svclist="remotetest.unidata.ucar.edu"
REMOTETESTSERVERS="$svclist"
fi
AC_MSG_RESULT([$svclist])
Expand Down
80 changes: 46 additions & 34 deletions libdap4/d4chunk.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ and whether it has checksums.
*/

/* Define a local struct for convenience */
struct HDR {unsigned int flags; unsigned int count;};
struct HDR {unsigned int flags; unsigned int count;};

/* Forward */
static void* getheader(void* p, struct HDR* hdr, int hostlittleendian);
Expand Down Expand Up @@ -60,15 +60,18 @@ NCD4_dechunk(NCD4meta* metadata)
q = metadata->serial.rawdata;
if(memcmp(q,"<?xml",strlen("<?xml"))==0
|| memcmp(q,"<Dataset",strlen("<Dataset"))==0) {
if(metadata->mode != NCD4_DMR)
return THROW(NC_EDMR);
/* setup as dmr only */
metadata->serial.dmr = (char*)metadata->serial.rawdata; /* temp */
metadata->serial.dmr[metadata->serial.rawsize-1] = '\0';
metadata->serial.dmr = strdup((char *)q);
if(metadata->serial.dmr == NULL)
return THROW(NC_ENOMEM);
return THROW(NC_NOERR);
if(metadata->mode != NCD4_DMR)
return THROW(NC_EDMR);
/* setup as dmr only */
metadata->serial.dmr = (char*)metadata->serial.rawdata; /* temp */
/* Avoid strdup since rawdata might contain nul chars */
if((metadata->serial.dmr = malloc(metadata->serial.rawsize+1)) == NULL)
return THROW(NC_ENOMEM);
memcpy(metadata->serial.dmr,metadata->serial.rawdata,metadata->serial.rawsize);
metadata->serial.dmr[metadata->serial.rawsize-1] = '\0';
/* Suppress nuls */
(void)NCD4_elidenuls(metadata->serial.dmr,metadata->serial.rawsize);
return THROW(NC_NOERR);
}

/* We must be processing a DAP mode packet */
Expand All @@ -82,7 +85,7 @@ NCD4_dechunk(NCD4meta* metadata)
/* Get the DMR chunk header*/
p = getheader(p,&hdr,metadata->serial.hostlittleendian);
if(hdr.count == 0)
return THROW(NC_EDMR);
return THROW(NC_EDMR);
if(hdr.flags & ERR_CHUNK) {
return processerrchunk(metadata, (void*)p, hdr.count);
}
Expand All @@ -91,31 +94,40 @@ NCD4_dechunk(NCD4meta* metadata)
metadata->localchecksumming = metadata->serial.remotechecksumming;

metadata->serial.remotelittleendian = ((hdr.flags & LITTLE_ENDIAN_CHUNK) ? 1 : 0);
metadata->serial.dmr = (char*)p;
/* Again, avoid strxxx operations on dmr */
if((metadata->serial.dmr = malloc(hdr.count+1)) == NULL)
return THROW(NC_ENOMEM);
memcpy(metadata->serial.dmr,p,hdr.count);
metadata->serial.dmr[hdr.count-1] = '\0';
metadata->serial.dmr = strdup(metadata->serial.dmr);
if(metadata->serial.dmr == NULL)
return THROW(NC_ENOMEM);
p += hdr.count;
/* Suppress nuls */
(void)NCD4_elidenuls(metadata->serial.dmr,hdr.count);

if(hdr.flags & LAST_CHUNK)
return THROW(NC_ENODATA);
/* Read and compress the data chunks */
q = metadata->serial.dap;
p = p + hdr.count; /* point to data chunk header */
/* Do a sanity check in case the server has shorted us with no data */
if((hdr.count + CHUNKHDRSIZE) >= metadata->serial.rawsize) {
/* Server only sent the DMR part */
metadata->serial.dapsize = 0;
return THROW(NC_EDATADDS);
}
q = metadata->serial.dap;
for(;;) {
p = getheader(p,&hdr,metadata->serial.hostlittleendian);
if(hdr.flags & ERR_CHUNK) {
p = getheader(p,&hdr,metadata->serial.hostlittleendian);
if(hdr.flags & ERR_CHUNK) {
return processerrchunk(metadata, (void*)p, hdr.count);
}
/* data chunk; possibly last; possibly empty */
if(hdr.count > 0) {
d4memmove(q,p,hdr.count); /* will overwrite the header */
p += hdr.count;
q += hdr.count;
}
if(hdr.flags & LAST_CHUNK) break;
}
/* data chunk; possibly last; possibly empty */
if(hdr.count > 0) {
d4memmove(q,p,hdr.count); /* will overwrite the header */
p += hdr.count;
q += hdr.count;
}
if(hdr.flags & LAST_CHUNK) break;
}
metadata->serial.dapsize = (size_t)DELTA(q,metadata->serial.dap);

#ifdef D4DUMPDMR
fprintf(stderr,"%s\n",metadata->serial.dmr);
fflush(stderr);
Expand Down Expand Up @@ -169,8 +181,8 @@ getheader(void* p, struct HDR* hdr, int hostlittleendian)
hyrax.count = *(unsigned int*)bytes; /* get count */
/* See which makes more sense */
if(hyrax.flags <= ALL_CHUNK_FLAGS && hyrax.count >= 0 && hyrax.count < hdr->count) {
/* Use hyrax version */
*hdr = hyrax;
/* Use hyrax version */
*hdr = hyrax;
}
#endif
return p;
Expand All @@ -187,17 +199,17 @@ NCD4_infermode(NCD4meta* meta)
char* raw = meta->serial.rawdata;

if(size < 16)
return THROW(NC_EDAP); /* must have at least this to hold a hdr + partial dmr*/
return THROW(NC_EDAP); /* must have at least this to hold a hdr + partial dmr*/
if(memcmp(raw,"<?xml",strlen("<?xml"))==0
|| memcmp(raw,"<Dataset",strlen("<Dataset"))==0) {
meta->mode = NCD4_DMR;
goto done;
meta->mode = NCD4_DMR;
goto done;
}
raw += 4; /* Pretend we have a DAP hdr */
if(memcmp(raw,"<?xml",strlen("<?xml"))==0
|| memcmp(raw,"<Dataset",strlen("<Dataset"))==0) {
meta->mode = NCD4_DAP;
goto done;
meta->mode = NCD4_DAP;
goto done;
}
/* Default to DSR */
meta->mode = NCD4_DSR;
Expand Down
16 changes: 16 additions & 0 deletions libdap4/d4util.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,22 @@ NCD4_entityescape(const char* s)
return escaped;
}

/* Elide all nul characters from an XML document as a precaution*/
size_t
NCD4_elidenuls(char* s, size_t slen)
{
size_t i,j;
for(j=0,i=0;i<slen;i++) {
int c = s[i];
if(c != 0)
s[j++] = (char)c;
}
/* if we remove any nuls then nul term */
if(j < i)
s[j] = '\0';
return j;
}

int
NCD4_readfile(const char* filename, NCbytes* content)
{
Expand Down
4 changes: 4 additions & 0 deletions libdap4/ncd4.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ defined here, including function-like #defines.
#define FIXEDOPAQUE
#define DFALTOPAQUESIZE 16

/* Size of a chunk header */
#define CHUNKHDRSIZE 4

/**************************************************/

#undef nullfree
Expand Down Expand Up @@ -137,6 +140,7 @@ extern char* NCD4_makeName(NCD4node*,const char* sep);
extern int NCD4_parseFQN(const char* fqn0, NClist* pieces);
extern char* NCD4_deescape(const char* esc);
extern char* NCD4_entityescape(const char* s);
extern size_t NCD4_elidenuls(char* s, size_t slen);

/* From d4dump.c */
extern void NCD4_dumpbytes(size_t size, const void* data0, int swap);
Expand Down
4 changes: 2 additions & 2 deletions libdispatch/derror.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -177,9 +177,9 @@ const char *nc_strerror(int ncerr1)
case NC_EDAS:
return "NetCDF: Malformed or inaccessible DAP DAS";
case NC_EDDS:
return "NetCDF: Malformed or inaccessible DAP DDS";
return "NetCDF: Malformed or inaccessible DAP2 DDS or DAP4 DMR response";
case NC_EDATADDS:
return "NetCDF: Malformed or inaccessible DAP DATADDS";
return "NetCDF: Malformed or inaccessible DAP2 DATADDS or DAP4 DAP response";
case NC_EDAPURL:
return "NetCDF: Malformed URL";
case NC_EDAPCONSTRAINT:
Expand Down
2 changes: 2 additions & 0 deletions libdispatch/dhttp.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,8 @@ setupconn(CURL* curl, const char* objecturl, NCbytes* buf)
if (cstat != CURLE_OK) goto fail;
cstat = CURLERR(curl_easy_setopt(curl, CURLOPT_NOPROGRESS, 1));
if (cstat != CURLE_OK) goto fail;
cstat = curl_easy_setopt(curl, CURLOPT_FOLLOWLOCATION, 1);
if (cstat != CURLE_OK) goto fail;

if(buf != NULL) {
/* send all data to this function */
Expand Down
2 changes: 2 additions & 0 deletions libhdf5/nc4info.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,8 @@ NC4_read_provenance(NC_FILE_INFO_T* file)
provenance->superblockversion = superblock;

/* Read the _NCProperties value from the file */
/* We do not return a size and assume the size is that upto the
first nul character */
if((ncstat = NC4_read_ncproperties(file,&propstring))) goto done;
provenance->ncproperties = propstring;
propstring = NULL;
Expand Down
3 changes: 2 additions & 1 deletion nc_test/tst_byterange.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ struct TESTURLS {
int format; /* instance of NC_FORMATX_XXX */
const char* url;
} testurls[] = {
{NC_FORMAT_CLASSIC,"http://149.165.169.123:8080/thredds/fileServer/testdata/2004050300_eta_211.nc#bytes"},
{NC_FORMAT_CLASSIC,"http://remotetest.unidata.ucar.edu/thredds/fileServer/testdata/2004050300_eta_211.nc#bytes"},
#ifdef USE_NETCDF4
{NC_FORMAT_NETCDF4,"http://noaa-goes16.s3.amazonaws.com/ABI-L1b-RadC/2017/059/03/OR_ABI-L1b-RadC-M3C13_G16_s20170590337505_e20170590340289_c20170590340316.nc#mode=bytes"},
#endif
Expand All @@ -59,6 +59,7 @@ dotest(struct TESTURLS* test)
int ncid;
int format = -1;

fprintf(stderr,"Test: url=%s\n",test->url);
/* First, try to open the url */
if((ret = nc_open(test->url,0,&ncid))) return fail(ret);

Expand Down

0 comments on commit f587654

Please sign in to comment.