Skip to content

Commit

Permalink
Yet another fix for DAP2 double URL encoding.
Browse files Browse the repository at this point in the history
re:  Unidata#1876
and: Unidata#1835
and: Unidata/netcdf4-python#1041

The change in PR 1835 was correct with respect to using %20 instead of '+'
for encoding blanks. However, it was a mistake to assume everything was
unencoded and then to do encoding ourselves. The problem is that
different servers do different things, with Columbia being an outlier.

So, I have added a set of client controls that can at least give
the caller some control over this. The caller can append
the following fragment to his URL to control what gets encoded before
sending it to the server. The syntax is as follows:
````
https://<host>/<path>/<query>#encode=path|query|all|none
````

The possible values:
* path  -- URL encode (i.e. %xx encode) as needed in the path part of the URL.
* query -- URL encode as needed in the query part of the URL.
* all   -- equivalent to ````#encode=path,query````.
* none  -- do not url encode any part of the URL sent to the server; not strictly necessary, so mostly for completeness.

Note that if "encode=" is used, then before it is processed, all encoding
is turned of so that ````#encode=path```` will only encode the path
and not the query.

The default is ````#encode=query````, so the path is left untouched,
but the query is always encoded.

Internally, this required changes to pass the encode flags down into
the OC2 library.

Misc. Unrelated Changes:
* Shut up those irritating warning from putget.m4
  • Loading branch information
DennisHeimbigner committed Nov 5, 2020
1 parent c8a3c51 commit 793ecc8
Show file tree
Hide file tree
Showing 15 changed files with 126 additions and 48 deletions.
1 change: 1 addition & 0 deletions RELEASE_NOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ This file contains a high-level description of this package's evolution. Release

## 4.8.0 - TBD

* [Enhancement] Give the client control over what parts of a DAP2 URL are URL encoded (i.e. %xx). This is to support the different decoding rules that servers apply to incoming URLS.
* [Bug Fix] Fix incorrect time offsets from `ncdump -t`, in some cases when the time `units` attribute contains both a **non-zero** time-of-day, and a time zone suffix containing the letter "T", such as "UTC". See [Github #1866](https://github.com/Unidata/netcdf-c/pull/1866) for more information.
* [Bug Fix] Cleanup the NCZarr S3 build options. See [Github #1869](https://github.com/Unidata/netcdf-c/pull/1869) for more information.
* [Bug Fix] Support aligned access for selected ARM processors. See [Github #1871](https://github.com/Unidata/netcdf-c/pull/1871) for more information.
Expand Down
8 changes: 4 additions & 4 deletions configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -498,11 +498,11 @@ CFLAGS="$SAVECFLAGS"
# --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)])
# Temporary: Change default to not do these tests
AC_MSG_CHECKING([whether dap remote testing should be enabled (default off)])
AC_ARG_ENABLE([dap-remote-tests],
[AS_HELP_STRING([--disable-dap-remote-tests],
[disable dap remote tests])])
[AS_HELP_STRING([--enable-dap-remote-tests],
[enable dap remote tests])])
test "x$enable_dap_remote_tests" = xyes || enable_dap_remote_tests=no
if test "x$enable_dap" = "xno" ; then
enable_dap_remote_tests=no
Expand Down
3 changes: 3 additions & 0 deletions include/ncdap.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ typedef unsigned int NCFLAGS;
#define NCF_PREFETCH_ALL (0x0800) /* Prefetch all variables */
/* Allow _FillValue/Variable type mismatch */
#define NCF_FILLMISMATCH (0x1000)
/* Hack to control URL encoding */
#define NCF_ENCODE_PATH (0x2000)
#define NCF_ENCODE_QUERY (0x4000)
/*COLUMBIA_HACK*/
#define NCF_COLUMBIA (0x80000000) /* Hack for columbia server */

Expand Down
19 changes: 10 additions & 9 deletions include/ncuri.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,16 @@
#include "ncexternl.h"

/* Define flags to control what is included by ncuribuild*/
#define NCURIPATH 1
#define NCURIPWD 2
#define NCURIQUERY 4
#define NCURIFRAG 8
#define NCURIENCODE 16 /* If output should be encoded */
#define NCURIBASE (NCURIPWD|NCURIPATH)
#define NCURISVC (NCURIQUERY|NCURIBASE) /* for sending to server */
#define NCURIALL (NCURIPATH|NCURIPWD|NCURIQUERY|NCURIFRAG) /* for rebuilding after changes */

#define NCURIPATH 1
#define NCURIPWD 2
#define NCURIQUERY 4
#define NCURIFRAG 8
#define NCURIENCODEPATH 16 /* If output url path should be encoded */
#define NCURIENCODEQUERY 32 /* If output url query should be encoded */
#define NCURIENCODE (NCURIENCODEPATH|NCURIENCODEQUERY)
#define NCURIBASE (NCURIPWD|NCURIPATH)
#define NCURISVC (NCURIQUERY|NCURIBASE) /* for sending to server */
#define NCURIALL (NCURIPATH|NCURIPWD|NCURIQUERY|NCURIFRAG) /* for rebuilding after changes */

/*! This is an open structure meaning
it is ok to directly access its fields
Expand Down
33 changes: 25 additions & 8 deletions libdap2/daputil.c
Original file line number Diff line number Diff line change
Expand Up @@ -654,8 +654,8 @@ dap_fetch(NCDAPCOMMON* nccomm, OClink conn, const char* ce,
NCerror ncstat = NC_NOERR;
OCerror ocstat = OC_NOERR;
char* ext = NULL;
OCflags flags = 0;
int httpcode = 0;
OCflags ocflags = 0;
#ifdef HAVE_GETTIMEOFDAY
struct timeval time0;
struct timeval time1;
Expand All @@ -668,13 +668,14 @@ dap_fetch(NCDAPCOMMON* nccomm, OClink conn, const char* ce,
if(ce != NULL && strlen(ce) == 0)
ce = NULL;

if(FLAGSET(nccomm->controls,NCF_UNCONSTRAINABLE)) {
if(FLAGSET(nccomm->controls,NCF_UNCONSTRAINABLE))
ce = NULL;
}

if(FLAGSET(nccomm->controls,NCF_ONDISK)) {
flags |= OCONDISK;
}
if(FLAGSET(nccomm->controls,NCF_ONDISK))
ocflags |= OCONDISK;
if(FLAGSET(nccomm->controls,NCF_ENCODE_PATH))
ocflags |= OCENCODEPATH;
if(FLAGSET(nccomm->controls,NCF_ENCODE_QUERY))
ocflags |= OCENCODEQUERY;

if(SHOWFETCH) {
/* Build uri string minus the constraint and #tag */
Expand All @@ -688,7 +689,7 @@ dap_fetch(NCDAPCOMMON* nccomm, OClink conn, const char* ce,
gettimeofday(&time0,NULL);
#endif
}
ocstat = oc_fetch(conn,ce,dxd,flags,rootp);
ocstat = oc_fetch(conn,ce,dxd,ocflags,rootp);
if(FLAGSET(nccomm->controls,NCF_SHOWFETCH)) {
#ifdef HAVE_GETTIMEOFDAY
double secs;
Expand Down Expand Up @@ -810,3 +811,19 @@ nccpadding(unsigned long offset, int alignment)
return pad;
}

int
dapparamparselist(const char* s0, int delim, NClist* list)
{
int stat = NC_NOERR;
char* s = strdup(s0);
char* p;
int i,count = 1;
if(s0 == NULL || strlen(s) == 0) goto done;
for(p=s;*p;p++) {if(*p == delim) {*p = '\0'; count++;}}
for(i=0,p=s;i<count;i++,p+=(strlen(p)+1)) {
if(strlen(p)>0)
nclistpush(list,strdup(p));
}
done:
return stat;
}
2 changes: 2 additions & 0 deletions libdap2/daputil.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,4 +72,6 @@ extern int dap_badname(char* name);
extern char* dap_repairname(char* name);
extern char* dap_getselection(NCURI* uri);

extern int dapparamparselist(const char* s0, int delim, NClist* list);

#endif /*DAPUTIL_H*/
28 changes: 28 additions & 0 deletions libdap2/ncd2dispatch.c
Original file line number Diff line number Diff line change
Expand Up @@ -2207,13 +2207,17 @@ fixzerodims(NCDAPCOMMON* dapcomm)
static void
applyclientparamcontrols(NCDAPCOMMON* dapcomm)
{
const char* value = NULL;

/* clear the flags */
CLRFLAG(dapcomm->controls,NCF_CACHE);
CLRFLAG(dapcomm->controls,NCF_SHOWFETCH);
CLRFLAG(dapcomm->controls,NCF_NC3);
CLRFLAG(dapcomm->controls,NCF_NCDAP);
CLRFLAG(dapcomm->controls,NCF_PREFETCH);
CLRFLAG(dapcomm->controls,NCF_PREFETCH_EAGER);
CLRFLAG(dapcomm->controls,NCF_ENCODE_PATH);
CLRFLAG(dapcomm->controls,NCF_ENCODE_QUERY);

/* Turn on any default on flags */
SETFLAG(dapcomm->controls,DFALT_ON_FLAGS);
Expand Down Expand Up @@ -2248,6 +2252,30 @@ applyclientparamcontrols(NCDAPCOMMON* dapcomm)
else if(dapparamcheck(dapcomm,"nofillmismatch",NULL))
CLRFLAG(dapcomm->controls,NCF_FILLMISMATCH);

if((value=dapparamvalue(dapcomm,"encode")) != NULL) {
int i;
NClist* encode = nclistnew();
if(dapparamparselist(value,',',encode))
nclog(NCLOGERR,"Malformed encode parameter: %s",value);
else {
/* First, turn off all the encode flags */
CLRFLAG(dapcomm->controls,NCF_ENCODE_PATH|NCF_ENCODE_QUERY);
for(i=0;i<nclistlength(encode);i++) {
char* s = nclistremove(encode,i);
if(strcmp(s,"path")==0)
SETFLAG(dapcomm->controls,NCF_ENCODE_PATH);
else if(strcmp(s,"query")==0)
SETFLAG(dapcomm->controls,NCF_ENCODE_QUERY);
else if(strcmp(s,"all")==0)
SETFLAG(dapcomm->controls,NCF_ENCODE_PATH|NCF_ENCODE_QUERY);
else if(strcmp(s,"none")==0)
CLRFLAG(dapcomm->controls,NCF_ENCODE_PATH|NCF_ENCODE_QUERY);
}
}
nclistfree(encode);
} else { /* Set defaults */
SETFLAG(dapcomm->controls,NCF_ENCODE_QUERY);
}
nclog(NCLOGNOTE,"Caching=%d",FLAGSET(dapcomm->controls,NCF_CACHE));

}
Expand Down
14 changes: 10 additions & 4 deletions libdispatch/ncuri.c
Original file line number Diff line number Diff line change
Expand Up @@ -629,7 +629,8 @@ ncuribuild(NCURI* duri, const char* prefix, const char* suffix, int flags)
{
char* newuri = NULL;
NCbytes* buf = ncbytesnew();
const int encode = (flags&NCURIENCODE ? 1 : 0);
const int encodepath = (flags&NCURIENCODEPATH ? 1 : 0);
const int encodequery = (flags&NCURIENCODEQUERY ? 1 : 0);

if(prefix != NULL)
ncbytescat(buf,prefix);
Expand All @@ -656,7 +657,7 @@ ncuribuild(NCURI* duri, const char* prefix, const char* suffix, int flags)
if((flags & NCURIPATH)) {
if(duri->path == NULL)
ncbytescat(buf,"/");
else if(encode) {
else if(encodepath) {
char* encoded = ncuriencodeonly(duri->path,pathallow);
ncbytescat(buf,encoded);
nullfree(encoded);
Expand All @@ -675,8 +676,13 @@ ncuribuild(NCURI* duri, const char* prefix, const char* suffix, int flags)
ensurequerylist(duri);
if(duri->query != NULL) {
ncbytescat(buf,"?");
ncbytescat(buf,duri->query);
}
if(encodequery) {
char* encoded = ncuriencodeonly(duri->query,queryallow);
ncbytescat(buf,encoded);
nullfree(encoded);
} else
ncbytescat(buf,duri->query);
}
}
if(flags & NCURIFRAG) {
ensurefraglist(duri);
Expand Down
4 changes: 4 additions & 0 deletions libsrc/putget.m4
Original file line number Diff line number Diff line change
Expand Up @@ -644,6 +644,8 @@ putNCvx_$1_$2(NC3_INFO* ncp, const NC_var *varp,
void *xp;
void *fillp=NULL;

NC_UNUSED(fillp);

if(nelems == 0)
return NC_NOERR;

Expand Down Expand Up @@ -847,7 +849,9 @@ getNCvx_$1_$2(const NC3_INFO* ncp, const NC_var *varp,
}
')dnl

#if 0 /*unused*/
GETNCVX(char, char)
#endif

GETNCVX(schar, schar)
GETNCVX(schar, short)
Expand Down
4 changes: 2 additions & 2 deletions oc2/oc.c
Original file line number Diff line number Diff line change
Expand Up @@ -1804,12 +1804,12 @@ oc_raw_xdrsize(OCobject link, OCobject ddsroot, off_t* xdrsizep)

/* Resend a url as a head request to check the Last-Modified time */
OCerror
oc_update_lastmodified_data(OCobject link)
oc_update_lastmodified_data(OCobject link, OCflags flags)
{
OCstate* state;
OCVERIFY(OC_State,link);
OCDEREF(OCstate*,state,link);
return OCTHROW(ocupdatelastmodifieddata(state));
return OCTHROW(ocupdatelastmodifieddata(state,flags));
}

long
Expand Down
13 changes: 11 additions & 2 deletions oc2/oc.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,18 @@ typedef int OCflags;
/*!\def OCONDISK
Cause oc_fetch to store the retrieved data on disk.
*/

#define OCONDISK 1

/*!\def OCENCODEPATH
Cause oc_fetch to encode path part of a URL
*/
#define OCENCODEPATH 2

/*!\def OCENCODEQUERY
Cause oc_fetch to encode query part of a URL
*/
#define OCENCODEQUERY 4

/**************************************************/
/* OCtype */

Expand Down Expand Up @@ -583,7 +592,7 @@ EXTERNL OCerror oc_set_curlopt(OClink link, const char* option, void* value);
EXTERNL OCerror oc_get_connection(OCobject ocnode, OCobject* linkp);

/* Resend a url as a head request to check the Last-Modified time */
EXTERNL OCerror oc_update_lastmodified_data(OClink);
EXTERNL OCerror oc_update_lastmodified_data(OClink,OCflags);

/* Get last known modification time; -1 => data unknown */
EXTERNL long oc_get_lastmodified_data(OClink);
Expand Down
11 changes: 7 additions & 4 deletions oc2/ocinternal.c
Original file line number Diff line number Diff line change
Expand Up @@ -180,14 +180,14 @@ ocfetch(OCstate* state, const char* constraint, OCdxd kind, OCflags flags,

switch (kind) {
case OCDAS:
stat = readDAS(state,tree);
stat = readDAS(state,tree,flags);
if(stat == OC_NOERR) {
tree->text = ncbytesdup(state->packet);
if(tree->text == NULL) stat = OC_EDAS;
}
break;
case OCDDS:
stat = readDDS(state,tree);
stat = readDDS(state,tree,flags);
if(stat == OC_NOERR) {
tree->text = ncbytesdup(state->packet);
if(tree->text == NULL) stat = OC_EDDS;
Expand Down Expand Up @@ -460,12 +460,15 @@ fprintf(stderr,"missing bod: ddslen=%lu bod=%lu\n",
}

OCerror
ocupdatelastmodifieddata(OCstate* state)
ocupdatelastmodifieddata(OCstate* state, OCflags ocflags)
{
OCerror status = OC_NOERR;
long lastmodified;
char* base = NULL;
base = ncuribuild(state->uri,NULL,NULL,NCURIENCODE);
int flags = 0;
if(ocflags & OCENCODEPATH) flags |= NCURIENCODEPATH;
if(ocflags & OCENCODEQUERY) flags |= NCURIENCODEQUERY;
base = ncuribuild(state->uri,NULL,NULL,flags);
status = ocfetchlastmodified(state->curl, base, &lastmodified);
free(base);
if(status == OC_NOERR) {
Expand Down
2 changes: 1 addition & 1 deletion oc2/ocinternal.h
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ extern int oc_network_order;
extern int oc_invert_xdr_double;
extern OCerror ocinternalinitialize(void);

extern OCerror ocupdatelastmodifieddata(OCstate* state);
extern OCerror ocupdatelastmodifieddata(OCstate* state, OCflags);

extern OCerror ocset_useragent(OCstate* state, const char* agent);
extern OCerror ocset_netrc(OCstate* state, const char* path);
Expand Down
Loading

0 comments on commit 793ecc8

Please sign in to comment.