Merge pull request #1998 from DennisHeimbigner/mktmp.dmh

Fix bug with windows version of mkstemp that causes failure to create more than 26 temp files

RELEASE_NOTES.md

@@ -7,7 +7,7 @@ This file contains a high-level description of this package's evolution. Release
 
 ## 4.8.1 - TBD
 
-
+* [Bug Fix] Fix bug with windows version of mkstemp that causes failure to create more than 26 temp files. See [Github #1998](https://github.com/Unidata/netcdf-c/pull/1998).  
 * [Bug Fix] Fix ncdump bug when printing VLENs with basetype char. See [Github #1986](https://github.com/Unidata/netcdf-c/issues/1986).  
 * [Bug Fixes] The netcdf-c library was incorrectly determining the scope of types referred to by nc_inq_type_equal. See [Github #1959](https://github.com/Unidata/netcdf-c/pull/1959) for more information.
 * [Bug Fix] Fix bug in use of XGetopt when building under Mingw. See [Github #2009](https://github.com/Unidata/netcdf-c/issues/2009).

dap4_test/test_hyrax.sh

@@ -28,16 +28,22 @@ failure() {
 
 setresultdir results_test_hyrax
 
+TESTSERVER=`${execdir}/findtestserver4 dap4 opendap test.opendap.org`
+if test "x$TESTSERVER" = x ; then
+echo "***XFAIL: Cannot find test.opendap.org testserver; test skipped"
+exit 0
+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:8080/opendap/${prefix}/${base}${FRAG}"
+        URL="dap4://test.opendap.org/opendap/${prefix}/${base}${FRAG}"
     else
-	URL="dap4://test.opendap.org:8080/opendap/${prefix}/${base}?$constraint${FRAG}"
+	URL="dap4://test.opendap.org/opendap/${prefix}/${base}?$constraint${FRAG}"
     fi
     echo "testing: $URL"
     if ! ${NCDUMP} "${URL}" > ./results_test_hyrax/${base}.hyrax; then

include/ncpathmgr.h

@@ -124,6 +124,8 @@ EXTERNL int NCremove(const char* path);
 EXTERNL int NCmkdir(const char* path, int mode);
 EXTERNL int NCrmdir(const char* path);
 EXTERNL char* NCgetcwd(char* cwdbuf, size_t len);
+EXTERNL int NCmkstemp(char* buf);
+
 #ifdef HAVE_SYS_STAT_H
 EXTERNL int NCstat(char* path, struct stat* buf);
 #endif
@@ -139,6 +141,7 @@ EXTERNL int NCclosedir(DIR* ent);
 #define NCaccess(path,mode) access(path,mode)
 #define NCmkdir(path,mode) mkdir(path,mode)
 #define NCgetcwd(buf,len) getcwd(buf,len)
+#define NCmkstemp(buf) mkstemp(buf);
 #define NCcwd(buf, len) getcwd(buf,len)
 #define NCrmdir(path) rmdir(path)
 #ifdef HAVE_SYS_STAT_H

libdap2/ncd2dispatch.c

@@ -1306,8 +1306,8 @@ applyclientparams(NCDAPCOMMON* nccomm)
 	strlcat(tmpname,pathstr,sizeof(tmpname));
 	value = paramlookup(nccomm,tmpname);
 	if(value == NULL) {
-	    strcpy(tmpname,"maxstrlen_");
-	    strncat(tmpname,pathstr,NC_MAX_NAME);
+	    strncpy(tmpname,"maxstrlen_",sizeof(tmpname));
+	    strlcat(tmpname,pathstr,sizeof(tmpname));
 	    value = paramlookup(nccomm,tmpname);
         }
 	nullfree(pathstr);

libdap4/d4parser.c

@@ -545,7 +545,7 @@ parseSequence(NCD4parser* parser, NCD4node* container, ezxml_t xml, NCD4node** n
 	vlentype->basetype = var->basetype;
 	/* Use name <fqnname>_t */
 	strncpy(name,fqnname,sizeof(name));
-	strncat(name,"_t", sizeof(name) - strlen(name) - 1);
+	strlcat(name,"_t", sizeof(name));
         SETNAME(vlentype,name);
         /* Set the basetype */
         var->basetype = vlentype;
@@ -560,7 +560,7 @@ parseSequence(NCD4parser* parser, NCD4node* container, ezxml_t xml, NCD4node** n
         classify(group,structtype);
 	/* Use name <fqnname>_base */
 	strncpy(name,fqnname,sizeof(name));
-	strncat(name,"_base", sizeof(name) - strlen(name) - 1);
+	strlcat(name,"_base", sizeof(name));
         SETNAME(structtype,name);
         /* Parse Fields into type */
         if((ret = parseFields(parser,structtype,xml))) goto done;
@@ -569,7 +569,7 @@ parseSequence(NCD4parser* parser, NCD4node* container, ezxml_t xml, NCD4node** n
         classify(group,vlentype);
 	/* Use name <xname>_t */
 	strncpy(name,fqnname,sizeof(name));
-	strncat(name,"_t", sizeof(name) - strlen(name) - 1);
+	strlcat(name,"_t", sizeof(name));
         SETNAME(vlentype,name);
 	vlentype->basetype = structtype;
         /* Set the basetype */

libdap4/d4util.c

@@ -383,7 +383,7 @@ NCD4_mktmp(const char* base, char** tmpnamep)
 
     strncpy(tmp,base,sizeof(tmp));
 #ifdef HAVE_MKSTEMP
-    strncat(tmp,"XXXXXX", sizeof(tmp) - strlen(tmp) - 1);
+    strlcat(tmp,"XXXXXX", sizeof(tmp));
     /* Note Potential problem: old versions of this function
        leave the file in mode 0666 instead of 0600 */
     mask=umask(0077);
@@ -396,7 +396,7 @@ NCD4_mktmp(const char* base, char** tmpnamep)
 	char spid[7];
 	if(rno < 0) rno = -rno;
         snprintf(spid,sizeof(spid),"%06d",rno);
-        strncat(tmp,spid,sizeof(tmp));
+        strlcat(tmp,spid,sizeof(tmp));
 #if defined(_WIN32) || defined(_WIN64)
         fd=open(tmp,O_RDWR|O_BINARY|O_CREAT, _S_IREAD|_S_IWRITE);
 #  else
@@ -417,12 +417,12 @@ void
 NCD4_hostport(NCURI* uri, char* space, size_t len)
 {
     if(space != NULL && len >  0) {
-	space[0] = '\0'; /* so we can use strncat */
+	space[0] = '\0'; /* so we can use strlcat */
         if(uri->host != NULL) {
-	    strncat(space,uri->host,len);
+	    strlcat(space,uri->host,len);
             if(uri->port != NULL) {
-	        strncat(space,":",len);
-	        strncat(space,uri->port,len);
+	        strlcat(space,":",len);
+	        strlcat(space,uri->port,len);
 	    }
 	}
     }
@@ -432,11 +432,11 @@ void
 NCD4_userpwd(NCURI* uri, char* space, size_t len)
 {
     if(space != NULL && len > 0) {
-	space[0] = '\0'; /* so we can use strncat */
+	space[0] = '\0'; /* so we can use strlcat */
         if(uri->user != NULL && uri->password != NULL) {
-            strncat(space,uri->user,len);
-            strncat(space,":",len);
-            strncat(space,uri->password,len);
+            strlcat(space,uri->user,len);
+            strlcat(space,":",len);
+            strlcat(space,uri->password,len);
 	}
     }
 }

libdispatch/dauth.c

@@ -81,8 +81,8 @@ NC_combinehostport(NCURI* uri)
     if(hp == NULL) return NULL;
     strncpy(hp,host,len);
     if(port != NULL) {
-	strncat(hp,":",len);
-	strncat(hp,port,len);
+	strlcat(hp,":",len+1);
+	strlcat(hp,port,len+1);
     }
     return hp;
 }

libdispatch/ddispatch.c

@@ -56,7 +56,7 @@ NCDISPATCH_initialize(void)
     /* Capture temp dir*/
     {
 	char* tempdir = NULL;
-#if defined _WIN32 || defined __MSYS__
+#if defined _WIN32 || defined __MSYS__ || defined __CYGWIN__
         tempdir = getenv("TEMP");
 #else
 	tempdir = "/tmp";

libdispatch/dpathmgr.c

@@ -457,6 +457,43 @@ NCgetcwd(char* cwdbuf, size_t cwdlen)
     return cwdbuf;
 }
 
+EXTERNL
+int
+NCmkstemp(char* base)
+{
+    int stat = 0;
+    int fd, rno;
+    char* tmp = NULL;
+    size_t len;
+    char* xp = NULL;
+    char* cvtpath = NULL;
+    int attempts;
+
+    cvtpath = NCpathcvt(base);
+    len = strlen(cvtpath);
+    xp = cvtpath+(len-6);
+    assert(memcmp(xp,"XXXXXX")==0);    
+    for(attempts=10;attempts>0;attempts--) {
+        /* The Windows version of mkstemp does not work right;
+           it only allows for 26 possible XXXXXX values */
+        /* Need to simulate by using some kind of pseudo-random number */
+        rno = rand();
+        if(rno < 0) rno = -rno;
+        snprintf(xp,7,"%06d",rno);
+        fd=NCopen3(cvtpath,O_RDWR|O_BINARY|O_CREAT, _S_IREAD|_S_IWRITE);
+        if(fd >= 0) break;
+    }
+    if(fd < 0) {
+       nclog(NCLOGERR, "Could not create temp file: %s",tmp);
+       stat = EACCES;
+       goto done;
+    }
+done:
+    nullfree(cvtpath);
+    if(stat && fd >= 0) {close(fd);}    
+    return (stat?-1:fd);
+}
+
 #ifdef HAVE_SYS_STAT_H
 EXTERNL
 int

libdispatch/drc.c

@@ -472,12 +472,12 @@ rcsearch(const char* prefix, const char* rcname, char** pathp)
     size_t rclen = strlen(rcname);
     int ret = NC_NOERR;
 
-    size_t pathlen = plen+rclen+1; /*+1 for '/' */
-    path = (char*)malloc(pathlen+1); /* +1 for nul*/
+    size_t pathlen = plen+rclen+1+1; /*+1 for '/' +1 for nul */
+    path = (char*)malloc(pathlen); /* +1 for nul*/
     if(path == NULL) {ret = NC_ENOMEM;	goto done;}
     strncpy(path,prefix,pathlen);
-    strncat(path,"/",pathlen);
-    strncat(path,rcname,pathlen);
+    strlcat(path,"/",pathlen);
+    strlcat(path,rcname,pathlen);
     /* see if file is readable */
     f = NCfopen(path,"r");
     if(f != NULL)

libdispatch/dutil.c

@@ -26,8 +26,6 @@
 #include "nclog.h"
 #include "ncpathmgr.h"
 
-extern int mkstemp(char *template);
-
 #define NC_MAX_PATH 4096
 
 #define LBRACKET '['
@@ -206,59 +204,23 @@ Return the generated path.
 char*
 NC_mktmp(const char* base)
 {
-    int fd;
-    char* cvtpath = NULL;
-    char tmp[NC_MAX_PATH];
-#ifdef HAVE_MKSTEMP
-    mode_t mask;
-#endif
-
-    /* Make sure that this path conversion has been applied
-       since we do not wrap mkstemp */
-    cvtpath = NCpathcvt(base);
-    strncpy(tmp,cvtpath,sizeof(tmp));
-    nullfree(cvtpath);
-	strncat(tmp, "XXXXXX", sizeof(tmp) - strlen(tmp) - 1);
+    int fd = -1;
+    char* tmp = NULL;
+    size_t len;
 
-#ifdef HAVE_MKSTEMP
-    /* Note Potential problem: old versions of this function
-       leave the file in mode 0666 instead of 0600 */
-    mask=umask(0077);
-    fd = mkstemp(tmp);
-    (void)umask(mask);
-#else /* !HAVE_MKSTEMP */
-    {
-#ifdef HAVE_MKTEMP
-#ifdef _MSC_VER
-        /* Use _mktemp_s */
-	_mktemp_s(tmp,sizeof(tmp)-1);
-#else /*!_MSC_VER*/
-        mktemp(tmp);
-	tmo[sizeof[tmp]-1] = '\0';
-#endif
-#else /* !HAVE_MKTEMP */
-	/* Need to simulate by using some kind of pseudo-random number */
-	{
-	    int rno = rand();
-	    char spid[7];
-	    if(rno < 0) rno = -rno;
-            snprintf(spid,sizeof(spid),"%06d",rno);
-            strncat(tmp,spid,sizeof(tmp) - strlen(tmp) - 1);
-	}
-#endif /* HAVE_MKTEMP */
-#ifdef _WIN32
-        fd=NCopen3(tmp,O_RDWR|O_BINARY|O_CREAT, _S_IREAD|_S_IWRITE);
-#else
-        fd=NCopen3(tmp,O_RDWR|O_CREAT|O_EXCL, S_IRWXU);
-#endif
-    }
-#endif /* !HAVE_MKSTEMP */
+    len = strlen(base)+6+1;
+    if((tmp = (char*)malloc(len))==NULL)
+        goto done;
+    strncpy(tmp,base,len);
+    strlcat(tmp, "XXXXXX", len);
+    fd = NCmkstemp(tmp);
     if(fd < 0) {
        nclog(NCLOGERR, "Could not create temp file: %s",tmp);
-       return NULL;
-    } else
-	close(fd);
-    return strdup(tmp);
+       goto done;
+    }
+done:
+    if(fd >= 0) close(fd);
+    return tmp;
 }
 
 int

ncdap_test/CMakeLists.txt

@@ -45,6 +45,7 @@ IF(ENABLE_TESTS)
       add_sh_test(ncdap tst_fillmismatch)
       IF(ENABLE_DAP_LONG_TESTS)
         add_sh_test(ncdap tst_longremote3)
+        add_bin_test(ncdap test_manyurls)
       ENDIF(ENABLE_DAP_LONG_TESTS)
 
     ENDIF(BUILD_UTILITIES)

ncdap_test/Makefile.am

@@ -52,13 +52,14 @@ TESTS += test_partvar
 
 if ENABLE_DAP_LONG_TESTS
  TESTS += tst_longremote3.sh
+ check_PROGRAMS += test_manyurls
+ TESTS += test_manyurls
 endif
 
 test_partvar_SOURCES = test_partvar.c
 
 t_misc_SOURCES = t_misc.c
 
-
 #TESTS += t_ncf330
 TESTS += t_misc
 

ncgen/cdata.c

@@ -104,11 +104,11 @@ c_constant(Generator* generator, Symbol* sym, NCConstant* con, Bytebuffer* buf,.
 	strcpy(special,"\"");
 	p = con->value.opaquev.stringv;
 	while(*p) {
-	    strcat(special,"\\x");
-	    strncat(special,p,2);	    	    
+	    strlcat(special,"\\x",bslen+3);
+	    strlcat(special,p,bslen+3);
 	    p += 2;	
 	}
-	strcat(special,"\"");
+	strlcat(special,"\"",bslen+3);
 	} break;
 
     default: PANIC1("ncstype: bad type code: %d",con->nctype);

ncgen/cmldata.c

@@ -337,9 +337,9 @@ xconst(Constant* ci)
 	bstring = poolalloc(bslen+2+1);
 	p = ci->value.opaquev.stringv;
 	while(*p) {
-	    strcat(bstring,"&#");
-	    strncat(bstring,p,2);
-	    strcat(bstring,";");
+	    strlcat(bstring,"&#",bslen+3);
+	    strlcat(bstring,p,bslen+3);
+	    strlcat(bstring,";",bslen+3);
 	    p += 2;
 	}
 	return bstring;

ncgen/generr.c

@@ -51,8 +51,8 @@ verror(const char *fmt, ...)
     char newfmt[2048];
     va_list argv;
     va_start(argv,fmt);
-    strcpy(newfmt,"netCDF classic: not supported: ");
-    strncat(newfmt,fmt,2000);
+    strncpy(newfmt,"netCDF classic: not supported: ",sizeof(newfmt));
+    strlcat(newfmt,fmt,sizeof(newfmt));
     vderror(newfmt,argv);
     va_end(argv);
 }

ncgen/genjjni.c

@@ -1503,11 +1503,11 @@ jconst(Constant* ci)
 	strcpy(bstring,"\"");
 	p = ci->value.opaquev.stringv;
 	while(*p) {
-	    strcat(bstring,"\\x");
-	    strncat(bstring,p,2);	    	    
+	    strlcat(bstring,"\\x",bslen+3);
+	    strlcat(bstring,p,bslen+3);
 	    p += 2;	
 	}
-	strcat(bstring,"\"");
+	strlcat(bstring,"\"",bslen+3);
 	return bstring;
 	} break;