Unifying OWS requests

[fschindler]

In this ticket an improvement to the current OWS request dispatching system shall be described.

Current situation

Currently, the dispatching algorithm is as follows:

• a CGI request object is parsed from either the HTTP/GET or HTTP/POST data
• the request handlers for WMS, WFS, WCS and SOS parse this request to their own parameters structures/variables. Every handler is dispatched (regardless of the request) until a handler is able to successfully handle it or an error is raised.
• in every handler, very similar methods are being used:
  • Parse the CGI request
  • determine the correctness of the parameters
  • determine if the requested version can be served
  • ... etc.

Proposed architecture

The proposed structure would be the following:

• before proceeding with the specific WxS/SOS handlers, the request is preliminarily parsed, in order to examine the version, the request operation and the service.
• this now allows to specifically call one request handler according to the requested service.
• In case of a POST/XML request (which is common for WCS/WFS/SOS) the request has to be parsed to a libxml2/CPLXML tree structure. This tree shall be saved for further processing and be forwarded to the request handler. This prevents the need of an additional parsing of the XML.

Benefits

The most important benefit would be a more unified treatment of the different OWS requests. This improves code modularity and the easy implementation of future extensions.

Another benefit is the better performance of the request parsing and processing, since the specific handler can be accessed directly and all other handlers are not bothered. Especially with XML requests the request parsing is a lot faster since the tree structure has to be built only once.

Downsides

The WxS/SOS request handlers have to be adjusted to fully incorporate the performance improvements this method grants.

[fschindler]

Addendum

I forgot to mention the fact, that some OWS do not require the service parameter (see #2531) for a list of all supported OWS and their parameters.

Patch ​r11840_ows_dispatch.patch

I also have prepared a patch to the mapows.c/.h files to adapt the changes I have proposed in the ticket. This patch only affects the msOWSDispatch process, but additional patches will follow for improvements in other WxS handlers.

The patch can be applied to the current trunk revision r11840.

Implementation Details

I used the barely used owsRequestObj struct to add the additional service, version and request parameters which are used by all OWS. Additionally, in the document field of the struct the parsed XML request is saved. I have chosen to use a void pointer because it then can be used as a libxml2 document or a CPLXMLNode to support XML requests, even if MapServer is compiled without libxml2.

To avoid the problem that WMS does not require the service parameter, I made a small check for the request parameter and set the service parameter to "WMS" if the request is either "GetMap" or "GetFeatureInfo". This part can be improved, if it does not satisfy the requirements.

[fschindler]

Patch ​r11840_wcs_dispatch.patch

This patch is also taken against the current trunk revision r11840.

This patch adapts the changes of this ticket and applies it on the WCS dispatching process. The dispatch of WCS 1.x and WCS 2.0 is now in a unified handler.

[schpidi]

Committed slightly adapted patches including updated tests and results with r12550.

[tbonfort]

the change causes mapows.c to not build if configured without --with-gdal or --with-libxml2 :

Undefined symbols for architecture x86_64:
  "_CPLParseXMLString", referenced from:
      _msOWSDispatch in libmapserver.a(mapows.o)
  "_CPLGetLastErrorMsg", referenced from:
      _msOWSDispatch in libmapserver.a(mapows.o)
  "_CPLStripXMLNamespace", referenced from:
      _msOWSDispatch in libmapserver.a(mapows.o)
  "_CPLGetXMLValue", referenced from:
      _msOWSDispatch in libmapserver.a(mapows.o)
  "_CPLDestroyXMLNode", referenced from:
      _msOWSDispatch in libmapserver.a(mapows.o)

[fschindler]

I can reproduce this issue and prepared a patch to address this issue.

Unfortunately I am not able to commit it, so I will post it here as an attachment. Hopefully someone will commit it by time. If not, I think Stephan will do so in the evening.

[fschindler]

Patch addressing the issue reported by tbonfort.

[schpidi]

Patch committed with r12570.

[rouault]

r12577 /trunk/mapserver/mapwcs20.c: Use CSLTokenizeString2() to avoid mismatch between MapServer's memory allocation routines and GDAL's CSLDestroy() used to free params->ids (potential problems if mapserver and gdal build with different MSVC runtimes) (fixes r12550, #3925)

Note (and advertisement ;-)) : The issue was detected because I use a custom build of GDAL for developement, where the #define DEBUG_VSIMALLOC line at the beginning of port/cpl_vsisimple.cpp is uncommented, so as to enable checks that try to detect (in)consistant use of GDAL memory allocation routines. When a mismatch is detected, an abort() is triggered, and then Valgrind can be used to easily track the cause for the mismatch.

In this case :

==16938== Invalid read of size 1
==16938==    at 0x4C29EAB: bcmp (mc_replace_strmem.c:679)
==16938==    by 0x75983E3: VSICheckMarker(char*) (cpl_vsisimple.cpp:471)
==16938==    by 0x7598667: VSIFree (cpl_vsisimple.cpp:550)
==16938==    by 0x756F754: CSLDestroy (cpl_string.cpp:180)
==16938==    by 0x54B108: msWCSFreeParamsObj20 (mapwcs20.c:344)
==16938==    by 0x52241C: msWCSDispatch (mapwcs.c:2268)
==16938==    by 0x4E6B3D: msOWSDispatch (mapows.c:289)
==16938==    by 0x466EF0: main (mapserv.c:1242)
==16938==  Address 0x192ef070 is 16 bytes before a block of size 5 alloc'd
==16938==    at 0x4C27878: malloc (vg_replace_malloc.c:236)
==16938==    by 0x10120D81: strdup (strdup.c:43)
==16938==    by 0x4830E1: msStrdup (mapstring.c:2117)
==16938==    by 0x54CE61: msWCSParseRequest20 (mapwcs20.c:1136)
==16938==    by 0x5220C8: msWCSDispatch (mapwcs.c:2197)
==16938==    by 0x4E6B3D: msOWSDispatch (mapows.c:289)
==16938==    by 0x466EF0: main (mapserv.c:1242)
==16938==

Convenient way to debug Windows issues from Linux ;-)

[schpidi]

r12655: mapscript OWSRequest->type is now initialized with MS_GET_REQUEST. Added check for request method in msOWSPreParseRequest.