Regularize the semantics of mkstemp.

re: https://github.com/Unidata/netcdf-c/issues/1827

The issue is partly resolved by this PR. The proximate problem appears to be that the semantics of mkstemp in **nix is different than the semantics of _mktemp_s in Windows. I had thought they were the same but that is incorrect. The _mktemp_s function will only produce 26 different files and so the netcdf temp file code will fail after about that many iterations.

So, to solve this, I created my own version of mkstemp for windows that uses a random number generator. This appears to solve the reported issue.  I also added the testcase ncdap_test/test_manyurls but made it conditional on --enable-dap-long-tests because it is very slow.

I did note that the provided test program now fails after some 800 iterations with a libcurl error claiming it cannot resolve the host name. My belief is that the library is just running out of resources at this point: too many open curl handles or some such. I doubt if this failure is fixable.

So bottom line is that it is really important to do nc_close when you are finished with a file.

Misc. Other Changes:

1. I took the opportunity to clean up some bad string hacks in the code. Specifically
    * change all uses of strncat to strlcat
    * remove old string hacks: occoncat and occopycat
2. Add heck to see if test.opendap.org is running and if not, then skip test
3. Make CYGWIN use TEMP environment variable
This commit is contained in:
Dennis Heimbigner 2021-05-14 11:33:03 -06:00
parent 44d9d365e9
commit 6901206927
23 changed files with 153 additions and 133 deletions

View File

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

View File

@ -28,6 +28,12 @@ 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`
@ -35,9 +41,9 @@ for f in $F ; do
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

View File

@ -118,6 +118,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
@ -133,6 +135,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);
#ifdef HAVE_SYS_STAT_H
#define NCstat(path,buf) stat(path,buf)
#endif

View File

@ -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);

View File

@ -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 */

View File

@ -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);
}
}
}

View File

@ -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;
}

View File

@ -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";

View File

@ -460,6 +460,43 @@ done:
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

View File

@ -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)

View File

@ -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
int fd = -1;
char* tmp = NULL;
size_t len;
/* 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);
#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

View File

@ -9,7 +9,7 @@
#undef ZDEBUG1 /* detailed debug */
#undef ZCATCH /* Warning: significant performance impact */
#define ZTRACING /* Warning: significant performance impact */
#undef ZTRACING /* Warning: significant performance impact */
#include "ncexternl.h"
#include "nclog.h"

View File

@ -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)

View File

@ -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

View File

@ -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);

View File

@ -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;

View File

@ -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);
}

View File

@ -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;

View File

@ -298,10 +298,11 @@ gen_load_c(
static void
fstrcat(
char *s, /* source string of stement being built */
const char *t, /* string to be appended to source */
size_t *slenp /* pointer to length of source string */
const char *t, /* string to be appended to source */
size_t *slenp /* pointer to length of source string */
)
{
size_t slen = *slenp;
*slenp += strlen(t);
@ -314,7 +315,11 @@ fstrcat(
/* Suppress a coverity-related issue without actually
ignoring it in the coverity dashboard. */
/* coverity[unsigned_compare] */
#if 0
strncat(s, t, MAX(0,MIN(strlen(t),strlen(s)-(strlen(t)))));
#else
strlcat(s, t, slen);
#endif
}
}

View File

@ -296,8 +296,7 @@ dumpfield(size_t index, char* n8, int isxdr)
snprintf(stmp,sizeof(stmp),"\\%02x",c);
else
snprintf(stmp,sizeof(stmp),"%c",c);
if(!occoncat(tmp,sizeof(tmp),1,stmp))
return;
strlcat(tmp,stmp,sizeof(tmp));
}
}
@ -605,9 +604,7 @@ ocdumpdatatree(OCstate* state, OCdata* data, NCbytes* buffer, int depth)
tabto(tabstops[++tabstop],buffer);
if(!occopycat(tmp,sizeof(tmp),1,pattern->name))
return;
ncbytescat(buffer,tmp);
ncbytescat(buffer,pattern->name);
if(rank > 0) {
snprintf(tmp,sizeof(tmp),"[%lu]",(unsigned long)crossproduct);

View File

@ -329,13 +329,16 @@ createtempfile(OCstate* state, OCtree* tree)
len =
strlen(globalstate->tempdir)
+ 1 /* '/' */
+ strlen(DATADDSFILE);
path = (char*)malloc(len+1);
+ strlen(DATADDSFILE)
+ 1; /* nul term */
path = (char*)malloc(len);
if(path == NULL) return OC_ENOMEM;
occopycat(path,len,3,globalstate->tempdir,"/",DATADDSFILE);
strncpy(path,globalstate->tempdir,len);
strlcat(path,"/",len);
strlcat(path,DATADDSFILE,len);
tmppath = NC_mktmp(path);
free(path);
if(stat != OC_NOERR) goto fail;
if(tmppath == NULL) {stat = OC_EACCESS; goto fail;}
#ifdef OCDEBUG
nclog(NCLOGNOTE,"oc_open: creating tmp file: %s",tmppath);
#endif
@ -530,11 +533,11 @@ ocset_curlproperties(OCstate* state)
if(state->auth->curlflags.useragent == NULL) {
size_t len = strlen(DFALTUSERAGENT) + strlen(VERSION) + 1;
char* agent = (char*)malloc(len+1);
if(occopycat(agent,len,2,DFALTUSERAGENT,VERSION))
state->auth->curlflags.useragent = agent;
else
free(agent);
char* agent = (char*)malloc(len);
strncpy(agent,DFALTUSERAGENT,len);
strlcat(agent,VERSION,len);
state->auth->curlflags.useragent = agent;
agent = NULL;
}
/* Some servers (e.g. thredds and columbia) appear to require a place
@ -546,31 +549,38 @@ ocset_curlproperties(OCstate* state)
state->auth->curlflags.cookiejar = NULL;
}
if(state->auth->curlflags.cookiejar == NULL) {
/* If no cookie file was defined, define a default */
int stat = NC_NOERR;
if (state->auth->curlflags.cookiejar == NULL) {
/* If no cookie file was defined, define a default */
int stat = NC_NOERR;
char* path = NULL;
char* tmppath = NULL;
int len;
errno = 0;
/* Create the unique cookie file name */
errno = 0;
/* Create the unique cookie file name */
len =
strlen(globalstate->tempdir)
+ 1 /* '/' */
+ strlen("occookies");
path = (char*)calloc(1,len+1);
if(path == NULL) return OC_ENOMEM;
occopycat(path,len,3,globalstate->tempdir,"/","occookies");
strlen(globalstate->tempdir)
+ 1 /* '/' */
+ strlen("occookies")
+ 1;
path = (char*)calloc(1, len);
if (path == NULL) return OC_ENOMEM;
strncpy(path,globalstate->tempdir,len);
strlcat(path,"/",len);
strlcat(path,"occookies",len);
tmppath = NC_mktmp(path);
if(tmppath == NULL) {
tmppath = NC_mktmp(path);
}
free(path);
state->auth->curlflags.cookiejar = tmppath;
state->auth->curlflags.cookiejarcreated = 1;
if(stat != OC_NOERR && errno != EEXIST) {
fprintf(stderr,"Cannot create cookie file\n");
goto fail;
}
errno = 0;
state->auth->curlflags.cookiejar = tmppath;
state->auth->curlflags.cookiejarcreated = 1;
if (stat != OC_NOERR && errno != EEXIST) {
fprintf(stderr, "Cannot create cookie file\n");
goto fail;
}
errno = 0;
}
OCASSERT(state->auth->curlflags.cookiejar != NULL);
/* Make sure the cookie jar exists and can be read and written */

View File

@ -214,8 +214,8 @@ readfile(const char* path, const char* suffix, NCbytes* packet)
char filename[1024];
/* check for leading file:/// */
if(ocstrncmp(path,"file://",7)==0) path += 7; /* assume absolute path*/
if(!occopycat(filename,sizeof(filename),2,path,(suffix != NULL ? suffix : "")))
return OCTHROW(OC_EOVERRUN);
strncpy(filename,path,sizeof(filename));
strlcat(filename,(suffix != NULL ? suffix : ""),sizeof(filename));
stat = NC_readfile(filename,packet);
return OCTHROW(stat);
}

View File

@ -558,25 +558,23 @@ ocdtmodestring(OCDT mode,int compact)
char* result = NULL;
int i;
char* p = NULL;
size_t len = 1+(NMODES*(MAXMODENAME+1));
result = malloc(1+(NMODES*(MAXMODENAME+1)));
result = malloc(len);
if(result == NULL) return NULL;
p = result;
result[0] = '\0';
if(mode == 0) {
if(compact) *p++ = '-';
else if(!occoncat(result,sizeof(result),1,"NONE"))
return NULL;
else strlcat(result,"NONE",len);
} else for(i=0;;i++) {
const char* ms = modestrings[i];
if(ms == NULL) break;
if(!compact && i > 0)
if(!occoncat(result,sizeof(result),1,","))
return NULL;
strlcat(result,";",len);
if(fisset(mode,(1<<i))) {
if(compact) *p++ = ms[0];
else if(!occoncat(result,sizeof(result),1,ms))
return NULL;
else strlcat(result,ms,len);
}
}
/* pad compact list out to NMODES in length (+1 for null terminator) */