Improve operation of the DAP4 code and fix bugs

.github/workflows/run_tests.yml

@@ -4,7 +4,7 @@
 
 name: Run netCDF Tests
 
-on: [pull_request]
+on: [pull_request,push]
 
 jobs:
 

RELEASE_NOTES.md

@@ -7,6 +7,7 @@ This file contains a high-level description of this package's evolution. Release
 
 ## 4.8.1 - TBD
 
+* [Bug Fix] Fix bugs in libdap4. See [Github #2005](https://github.com/Unidata/netcdf-c/issues/2005).
 * [Bug Fix] Store NCZarr fillvalue as a singleton instead of a 1-element array. See [Github #2017](https://github.com/Unidata/netcdf-c/issues/2017).  
 * [Bug Fixes] The netcdf-c library was incorrectly determining the scope of dimension; similar to the type scope problem. See [Github #2012](https://github.com/Unidata/netcdf-c/pull/2012) for more information.
 * [Bug Fix] Re-enable DAP2 authorization testing. See [Github #2011](https://github.com/Unidata/netcdf-c/issues/2011).  

dap4_test/CMakeLists.txt

@@ -13,6 +13,7 @@ ADD_SUBDIRECTORY(baseline)
 ADD_SUBDIRECTORY(baselineraw)
 ADD_SUBDIRECTORY(baselineremote)
 ADD_SUBDIRECTORY(baselinehyrax)
+ADD_SUBDIRECTORY(baselinethredds)
 ADD_SUBDIRECTORY(cdltestfiles)
 ADD_SUBDIRECTORY(daptestfiles)
 ADD_SUBDIRECTORY(dmrtestfiles)
@@ -55,6 +56,7 @@ IF(ENABLE_TESTS)
 
   IF(ENABLE_DAP_REMOTE_TESTS)
       add_sh_test(dap4_test test_hyrax)
+      add_sh_test(dap4_test test_thredds)
   ENDIF(ENABLE_DAP_REMOTE_TESTS)
 
 ENDIF(ENABLE_TESTS)

dap4_test/Makefile.am

@@ -42,18 +42,18 @@ pingurl4_SOURCES = pingurl4.c
 if ENABLE_DAP_REMOTE_TESTS
 if BUILD_UTILITIES
   # relies on ncdump
-  TESTS += test_hyrax.sh
+  TESTS += test_hyrax.sh test_thredds.sh
 if AX_IGNORE
   TESTS += test_remote.sh
 endif
 endif
 endif
 
 EXTRA_DIST = test_parse.sh test_meta.sh test_data.sh \
-             test_raw.sh test_remote.sh test_hyrax.sh test_fillmismatch.sh \
+             test_raw.sh test_remote.sh test_hyrax.sh test_thredds.sh test_fillmismatch.sh \
              tst_curlopt.sh d4test_common.sh \
 	     daptestfiles dmrtestfiles cdltestfiles nctestfiles misctestfiles \
-	     baseline baselineraw baselineremote baselinehyrax CMakeLists.txt test_common.h
+	     baseline baselineraw baselineremote baselinehyrax baselinethredds CMakeLists.txt test_common.h
 
 CLEANFILES = *.exe
 # This should only be left behind if using parallel io
@@ -72,8 +72,7 @@ clean-local: clean-local-check
 .PHONY: clean-local-check
 
 clean-local-check:
-	-rm -rf results results_test_parse results_test_data \
-                results_test_hyrax results_test_meta results_test_raw
+	-rm -rf results results_test_*
 	-rm -f .dodsrc .daprc
 
 # The shell file maketests.sh is used to build the testdata

dap4_test/baselinethredds/CMakeLists.txt

@@ -0,0 +1,13 @@
+# Copyright 1993, 1994, 1995, 1996, 1997, 1998, 1999, 2000, 2001, 2002,
+# 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011, 2012, 2013, 2014,
+# 2015, 2016, 2017, 2018
+# University Corporation for Atmospheric Research/Unidata.
+
+# See netcdf-c/COPYRIGHT file for more info.
+
+FILE(GLOB COPY_FILES ${CMAKE_CURRENT_SOURCE_DIR}/*)
+FILE(COPY ${COPY_FILES} DESTINATION ${CMAKE_CURRENT_BINARY_DIR}/ FILE_PERMISSIONS OWNER_READ OWNER_WRITE OWNER_EXECUTE)
+
+FILE(GLOB CUR_EXTRA_DIST RELATIVE ${CMAKE_CURRENT_SOURCE_DIR} ${CMAKE_CURRENT_SOURCE_DIR}/*)
+SET(CUR_EXTRA_DIST ${CUR_EXTRA_DIST} CMakeLists.txt)
+ADD_EXTRA_DIST("${CUR_EXTRA_DIST}")

dap4_test/d4test_common.sh

@@ -24,7 +24,7 @@ cd ${srcdir}/baseline; BASELINE=`pwd` ; cd ${WD}
 cd ${srcdir}/baselineraw; BASELINERAW=`pwd` ; cd ${WD}
 cd ${srcdir}/baselineremote; BASELINEREM=`pwd` ; cd ${WD}
 cd ${srcdir}/baselinehyrax; BASELINEH=`pwd` ; cd ${WD}
-#cd ${srcdir}/baselinethredds; BASELINETH=`pwd` ; cd ${WD}
+cd ${srcdir}/baselinethredds; BASELINETH=`pwd` ; cd ${WD}
 
 setresultdir() {
 rm -fr ${builddir}/$1
@@ -55,6 +55,23 @@ filesexist() {
     done
 }
 
+urlbasename() {
+    constraint=`echo "$1" | cut -d '?' -f2`
+    unconstrained=`echo "$1" | cut -d '?' -f1`
+    base=`basename $unconstrained`
+    prefix=`dirname $unconstrained`
+}
+
+makeurl() {
+    urlbasename "$2"
+    if test "x$constraint" = "x$unconstrained" ; then
+        URL="$1/${prefix}/${base}${FRAG}"
+    else
+	URL="$1/${prefix}/${base}?$constraint${FRAG}"
+    fi
+}
+
+
 finish() {
 if test "x$FAILURES" = x1 ; then
 echo "*** Fail"

dap4_test/test_common.h

@@ -105,9 +105,10 @@ setup(int tdmr, int argc, char** argv)
     if(translatenc4)
 	controller->controls.translation = NCD4_TRANSNC4;
     NCD4_applyclientparamcontrols(controller);
-    if((metadata=NCD4_newmeta(controller, ncbyteslength(input),ncbytescontents(input)))==NULL)
+    if((metadata=NCD4_newmeta(controller))==NULL)
 	fail(NC_ENOMEM);
     metadata->mode = mode;
+    NCD4_attachraw(metadata, ncbyteslength(input),ncbytescontents(input));
 
     if((ret=NCD4_dechunk(metadata))) /* ok for mode == DMR or mode == DAP */
 	fail(ret);

dap4_test/test_hyrax.sh

@@ -3,7 +3,6 @@
 if test "x$srcdir" = "x"; then srcdir=`dirname $0`; fi
 export srcdir;
 
-if test "x$srcdir" = x ; then srcdir=`pwd`; fi 
 . ../test_common.sh
 
 . ${srcdir}/d4test_common.sh
@@ -36,15 +35,9 @@ fi
 
 if test "x${RESET}" = x1 ; then rm -fr ${BASELINEH}/*.hyrax ; fi
 for f in $F ; do
-    constraint=`echo "$f" | cut -d '?' -f2`
-    unconstrained=`echo "$f" | cut -d '?' -f1`
-    base=`basename $unconstrained`
-    prefix=`dirname $unconstrained`
-    if test "x$constraint" = "x$unconstrained" ; then
-        URL="dap4://test.opendap.org/opendap/${prefix}/${base}${FRAG}"
-    else
-	URL="dap4://test.opendap.org/opendap/${prefix}/${base}?$constraint${FRAG}"
-    fi
+
+    makeurl "dap4://test.opendap.org/opendap" "$f"
+
     echo "testing: $URL"
     if ! ${NCDUMP} "${URL}" > ./results_test_hyrax/${base}.hyrax; then
         failure "${URL}"

dap4_test/test_thredds.sh

@@ -0,0 +1,49 @@
+#!/bin/sh
+
+if test "x$srcdir" = "x"; then srcdir=`dirname $0`; fi
+export srcdir;
+
+. ../test_common.sh
+
+. ${srcdir}/d4test_common.sh
+
+
+set -e
+echo "test_thredds.sh:"
+
+FRAG="#checksummode=ignore"
+
+F="\
+harvey/goes16/CONUS/Channel01/20170821/GOES16_CONUS_20170821_020218_0.47_1km_33.3N_91.4W.nc4?dap4.ce=y \
+"
+
+FBAD="harvey/model/gfs_ana/GFS_Global_0p5deg_ana_20170820_0000.grib2 \
+      harvey/nexrad/KCRP/20170820/Level2_KCRP_20170820_020558.ar2v"
+
+failure() {
+      echo "*** Fail: $1"
+      exit 1
+}
+
+setresultdir results_test_thredds
+
+if test "x${RESET}" = x1 ; then rm -fr ${BASELINEH}/*.thredds ; fi
+for f in $F ; do
+    makeurl "dap4://thredds-test.unidata.ucar.edu/thredds/dap4/casestudies" "$f"
+    echo "testing: $URL"
+    if ! ${NCDUMP} "${URL}" > ./results_test_thredds/${base}.thredds; then
+        failure "${URL}"
+    fi
+    if test "x${TEST}" = x1 ; then
+	if ! diff -wBb ${BASELINETH}/${base}.thredds ./results_test_thredds/${base}.thredds ; then
+	    failure "diff ${base}.thredds"
+	fi
+    elif test "x${RESET}" = x1 ; then
+	echo "${f}:" 
+	cp ./results_test_thredds/${base}.thredds ${BASELINETH}/${base}.thredds
+    fi
+done
+
+echo "*** Pass"
+exit 0
+

libdap4/d4chunk.c

@@ -47,38 +47,46 @@ NCD4_dechunk(NCD4meta* metadata)
     unsigned char *praw, *phdr, *pdap;
     NCD4HDR hdr;
 
-    if(metadata->mode == NCD4_DSR) 
-        return THROW(NC_EDMR);
-
-    /* Assume proper mode has been inferred already. */
-
-    /* Verify the mode; assume that the <?xml...?> is optional */
-    praw = metadata->serial.rawdata;
-    if(memcmp(praw,"<?xml",strlen("<?xml"))==0
-       || memcmp(praw,"<Dataset",strlen("<Dataset"))==0) {
-	size_t len = 0;
-        if(metadata->mode != NCD4_DMR) 
-            return THROW(NC_EDMR);
-        /* setup as dmr only */
-        /* Avoid strdup since rawdata might contain nul chars */
-	len = metadata->serial.rawsize;
-        if((metadata->serial.dmr = malloc(len+1)) == NULL)
-            return THROW(NC_ENOMEM);    
-        memcpy(metadata->serial.dmr,praw,len);
-        metadata->serial.dmr[len] = '\0';
-        /* Suppress nuls */
-        (void)NCD4_elidenuls(metadata->serial.dmr,len);
-        return THROW(NC_NOERR); 
-    }
-
 #ifdef D4DUMPRAW
     NCD4_tagdump(metadata->serial.rawsize,metadata->serial.rawdata,0,"RAW");
 #endif
 
+    /* Access the returned raw data */
+    praw = metadata->serial.rawdata;
+
+    if(metadata->mode == NCD4_DSR) {
+	return THROW(NC_EDMR);
+    } else if(metadata->mode == NCD4_DMR) {
+        /* Verify the mode; assume that the <?xml...?> is optional */
+        if(memcmp(praw,"<?xml",strlen("<?xml"))==0
+           || memcmp(praw,"<Dataset",strlen("<Dataset"))==0) {
+	    size_t len = 0;
+	    /* setup as dmr only */
+            /* Avoid strdup since rawdata might contain nul chars */
+	    len = metadata->serial.rawsize;
+            if((metadata->serial.dmr = malloc(len+1)) == NULL)
+                return THROW(NC_ENOMEM);    
+            memcpy(metadata->serial.dmr,praw,len);
+            metadata->serial.dmr[len] = '\0';
+            /* Suppress nuls */
+            (void)NCD4_elidenuls(metadata->serial.dmr,len);
+            return THROW(NC_NOERR); 
+	}
+    } else if(metadata->mode != NCD4_DAP)
+    	return THROW(NC_EDAP);
+
     /* We must be processing a DAP mode packet */
     praw = (metadata->serial.dap = metadata->serial.rawdata);
     metadata->serial.rawdata = NULL;
 
+    /* If the raw data looks like xml, then we almost certainly have an error */
+    if(memcmp(praw,"<?xml",strlen("<?xml"))==0
+           || memcmp(praw,"<!doctype",strlen("<!doctype"))==0) {
+	/* Set up to report the error */
+	int stat = NCD4_seterrormessage(metadata, metadata->serial.rawsize, metadata->serial.rawdata);
+        return THROW(stat); /* slight lie */
+    }
+
     /* Get the DMR chunk header*/
     phdr = NCD4_getheader(praw,&hdr,metadata->serial.hostlittleendian);
     if(hdr.count == 0)

libdap4/d4file.c

@@ -150,14 +150,16 @@ NCD4_open(const char * path, int mode,
     /* Always start by reading the DMR only */
     /* reclaim substrate.metadata */
     resetInfoforRead(d4info);
+    /* Rebuild metadata */
+    if((d4info->substrate.metadata=NCD4_newmeta(d4info))==NULL)
+	{ret = NC_ENOMEM; goto done;}
 
     if((ret=NCD4_readDMR(d4info, d4info->controls.flags.flags))) goto done;
 
-    /* (Re)Build the meta data; sets serial.rawdata */
+    /* set serial.rawdata */
     len = ncbyteslength(d4info->curl->packet);
     contents = ncbytesextract(d4info->curl->packet);
-    if((d4info->substrate.metadata=NCD4_newmeta(d4info, len, contents))==NULL)
-	{ret = NC_ENOMEM; goto done;}
+    NCD4_attachraw(d4info->substrate.metadata, len, contents);
 
     meta = d4info->substrate.metadata;
 

libdap4/d4http.c

@@ -87,7 +87,7 @@ NCD4_fetchurl_file(CURL* curl, const char* url, FILE* stream,
 }
 
 int
-NCD4_fetchurl(CURL* curl, const char* url, NCbytes* buf, long* filetime)
+NCD4_fetchurl(CURL* curl, const char* url, NCbytes* buf, long* filetime, int* httpcodep)
 {
     int ret = NC_NOERR;
     CURLcode cstat = CURLE_OK;
@@ -97,12 +97,12 @@ NCD4_fetchurl(CURL* curl, const char* url, NCbytes* buf, long* filetime)
     /* send all data to this function  */
     cstat = curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, WriteMemoryCallback);
     if (cstat != CURLE_OK)
-        goto fail;
+        goto done;
 
     /* we pass our file to the callback function */
     cstat = curl_easy_setopt(curl, CURLOPT_WRITEDATA, (void*)buf);
     if (cstat != CURLE_OK)
-        goto fail;
+        goto done;
 
     /* One last thing; always try to get the last modified time */
     cstat = curl_easy_setopt(curl, CURLOPT_FILETIME, (long)1);
@@ -111,7 +111,7 @@ NCD4_fetchurl(CURL* curl, const char* url, NCbytes* buf, long* filetime)
     cstat = curl_easy_setopt(curl, CURLOPT_URL, (void*)"");
     cstat = curl_easy_setopt(curl, CURLOPT_URL, (void*)url);
     if (cstat != CURLE_OK)
-        goto fail;
+        goto done;
 
     cstat = curl_easy_perform(curl);
 
@@ -121,14 +121,15 @@ NCD4_fetchurl(CURL* curl, const char* url, NCbytes* buf, long* filetime)
                curl_easy_strerror(cstat));
         cstat = CURLE_OK;
     }
-    httpcode = NCD4_fetchhttpcode(curl);
+    if(cstat != CURLE_OK) goto done;
 
-    if(cstat != CURLE_OK) goto fail;
+    httpcode = NCD4_fetchhttpcode(curl);
+    if(httpcodep) *httpcodep = httpcode;
 
     /* Get the last modified time */
     if(filetime != NULL)
         cstat = curl_easy_getinfo(curl,CURLINFO_FILETIME,filetime);
-    if(cstat != CURLE_OK) goto fail;
+    if(cstat != CURLE_OK) goto done;
 
     /* Null terminate the buffer*/
     len = ncbyteslength(buf);
@@ -137,19 +138,19 @@ NCD4_fetchurl(CURL* curl, const char* url, NCbytes* buf, long* filetime)
 #ifdef D4DEBUG
     nclog(NCLOGNOTE,"buffersize: %lu bytes",(d4size_t)ncbyteslength(buf));
 #endif
-    return THROW(ret);
 
-fail:
+done:
     if(cstat != CURLE_OK) {
         nclog(NCLOGERR, "curl error: %s", curl_easy_strerror(cstat));
         ret = curlerrtoncerr(cstat);
     } else switch (httpcode) {
-    case 401: ret = NC_EACCESS; break;
-    case 403: ret = NC_EAUTH; break;
-    case 404: ret = ENOENT; break;
-    case 500: ret = NC_EDAPSVC; break;
-    case 200: break;
-    default: ret = NC_ECURL; break;
+           case 400: ret = NC_EDATADAP; break;
+           case 401: ret = NC_EACCESS; break;
+	   case 403: ret = NC_EAUTH; break;
+           case 404: ret = ENOENT; break;
+           case 500: ret = NC_EDAPSVC; break;
+           case 200: break;
+           default: ret = NC_ECURL; break;
     }
     return THROW(ret);
 }
@@ -291,7 +292,7 @@ NCD4_ping(const char* url)
 
     /* Try to get the file */
     buf = ncbytesnew();
-    ret = NCD4_fetchurl(curl,url,buf,NULL);
+    ret = NCD4_fetchurl(curl,url,buf,NULL,NULL);
     if(ret == NC_NOERR) {
         /* Don't trust curl to return an error when request gets 404 */
         long http_code = 0;