Merging of WMS requests creates badly formed STYLES=,,,,, parameter

[mapserver-bot]

Reporter: jdoyon@nrcan.gc.ca
Date: 2006/03/09 - 19:41
Trac URL: http://trac.osgeo.org/mapserver/ticket/1707

In tracking down some problems, I discovered that for some reason WMS requests
generated by a mapserver WMS client tend to generate a badly formed STYLES
parameter that might look like this:

...&STYLES=,,,,,&...

The number of commas does not match the number of layers, and this breaks the
spec which says it should be empty, but always provided, since it's mandatory:

...&STYLES=&...

The problem comes from mapwmslayer.c:

Index: mapwmslayer.c
===================================================================
--- mapwmslayer.c       (revision 1010)
+++ mapwmslayer.c       (working copy)
@@ -959,7 +959,7 @@
         {
             value1 = msLookupHashTable(psLastWMSParams->params, keys[i]);
             value2 = msLookupHashTable(sThisWMSParams.params, keys[i]);
-            if (value1 && value2)
+            if (value1 && value2 && (value1 != "" || value2 != ""))
             {
                 char *pszBuf;
                 int nLen;

The problem is the concatenation of parameter values occurs even when said
parameter values are an empty string.  So the number of commas in STYLES= is
actually the number of mapserver layers (not wms ones) being concatenated into a
single one.

The above is my quick fix for it ... may not be the best, but seems to work for now.

[mapserver-bot]

Author: assefa
Date: 2006/03/10 - 00:20

looking into this

[mapserver-bot]

Author: assefa
Date: 2006/03/10 - 03:22

JF,

  Is it possible to post your map file (or part of it to reproduce the problem),
I would like to see how the styles are generated. Normally empty styles should
be filtered out.  

[mapserver-bot]

Author: jdoyon@nrcan.gc.ca
Date: 2006/03/10 - 15:55

I will send the MapFile seperately, as I don't want some of it's contents public
right now ...

What I saw was this:

    if (pszStyle==NULL)
    {
        /* When no style is selected, use "" which is a valid default. */
        pszStyle = "";
    }

Which led me to believe that value1 && value2 evaluated to true ... think maybe
C sees it that way?  I did this fix on a dev server and it seemed to help.  On a
different server however it doesn't.  I have to investigate further.

Here's part of a request as generated by MapServer:

LAYERS=SENTIER%5F1%3ABNDT%5F50K%2FNTDB%5F50K%2CROUTE%5FACCES%5FLIMIT%5F1%3ABNDT%5F50K%2FNTDB%5F50K,PERCEE%5F1%3ABNDT%5F50K%2FNTDB%5F50K,CHEMIN%5FDE%5FFER%5F1%3ABNDT%5F50K%2FNTDB%5F50K,PIPELINE%5F1%3ABNDT%5F50K%2FNTDB%5F50K%2CLIGN%5FTRANSMISSION%5F1%3ABNDT%5F50K%2FNTDB%5F50K,ESKER%5F1%3ABNDT%5F50K%2FNTDB%5F50K,INDEX%5FTHICK%5F50K,POINT%5FD%5FELEVATION%5F0%3ABNDT%5F50K%2FNTDB%5F50K,PLAQUE%5FTOURNANTE%5F0%3ABNDT%5F50K%2FNTDB%5F50K,POST%5FTRANSFRMTRS%5F0%3ABNDT%5F50K%2FNTDB%5F50K%2CVALVE%5F0%3ABNDT%5F50K%2FNTDB%5F50K%2CPOST%5FTRANSFRMTRS%5FANNOTAT%5F2%3ABNDT%5F50K%2FNTDB%5F50K,EOLIENNE%5F0%3ABNDT%5F50K%2FNTDB%5F50K%2CPUITS%5F0%3ABNDT%5F50K%2FNTDB%5F50K%2CRESERVOIR%5FSOUTERN%5F0%3ABNDT%5F50K%2FNTDB%5F50K%2CTOUR%5F0%3ABNDT%5F50K%2FNTDB%5F50K%2CCITERNE%5F0%3ABNDT%5F50K%2FNTDB%5F50K%2CSAUT%5FDE%5FSKI%5F0%3ABNDT%5F50K%2FNTDB%5F50K%2CSILO%5F0%3ABNDT%5F50K%2FNTDB%5F50K%2CPISTE%5FD%5FENVOL%5F0%3ABNDT%5F50K%2FNTDB%5F50K%2CRUINES%5F0%3ABNDT%5F50K%2FNTDB%5F50K%2CDEPOT%5FDE%5FLIQUIDE%5F0%3ABNDT%5F50K%2FNTDB%5F50K%2CGRUE%5F0%3ABNDT%5F50K%2FNTDB%5F50K%2CBATIMENT%5F0%3ABNDT%5F50K%2FNTDB%5F50K%2CCIMETIERE%5F2%5FANNOTAT%3ABNDT%5F50K%2FNTDB%5F50K%2CRUINES%5FANNOTAT%5F2%3ABNDT%5F50K%2FNTDB%5F50K%2CBATIMENT%5FANNOTAT%5F2%3ABNDT%5F50K%2FNTDB%5F50K,CENTRE%5FDE%5FSKI%5F0%3ABNDT%5F50K%2FNTDB%5F50K%2CLIEU%5FPELRNG%5F0%3ABNDT%5F50K%2FNTDB%5F50K%2CBASE%5FD%5FHYDRAVIONS%5F0%3ABNDT%5F50K%2FNTDB%5F50K%2CTERRN%5FPIQ%5FNIQ%5F0%3ABNDT%5F50K%2FNTDB%5F50K%2CZON%5FEXTRCTN%5FMINR%5F0%3ABNDT%5F50K%2FNTDB%5F50K%2CLIEU%5FHISTRQ%5FINTER%5F0%3ABNDT%5F50K%2FNTDB%5F50K%2CTERRN%5FPRATQ%5FGOLF%5F0%3ABNDT%5F50K%2FNTDB%5F50K%2CECHLL%5FPOISSNS%5F0%3ABNDT%5F50K%2FNTDB%5F50K%2CCINE%5FPARC%5F0%3ABNDT%5F50K%2FNTDB%5F50K%2CCIMETIERE%5F0%3ABNDT%5F50K%2FNTDB%5F50K%2CTERRAIN%5FCAMPING%5F0%3ABNDT%5F50K%2FNTDB%5F50K%2CCIMETIERE%5F2%5FANNOTAT%3ABNDT%5F50K%2FNTDB%5F50K%2CINSTLLTNS%5FGAZ%5FPET%5F0%3ABNDT%5F50K%2FNTDB%5F50K%2CTERRAIN%5FDE%5FGOLF%5FANNOTAT%5F2%3ABNDT%5F50K%2FNTDB%5F50K%2CTERRAIN%5FCAMPING%5FANNOTAT%5F2%3ABNDT%5F50K%2FNTDB%5F50K%2CTERRN%5FPRATQ%5FGOLF%5FANNOTAT%5F2%3ABNDT%5F50K%2FNTDB%5F50K%2CINSTLLTNS%5FGAZ%5FPET%5FANNOTAT%5F2%3ABNDT%5F50K%2FNTDB%5F50K%2CZON%5FEXTRCTN%5FMINR%5FINCONNU%5FANNOTAT%5F2%3ABNDT%5F50K%2FNTDB%5F50K%2CCINE%5FPARC%5F2%5FANNOTAT%3ABNDT%5F50K%2FNTDB%5F50K%2CTERRN%5FPIQ%5FNIQ%5FANNOTAT%5F2%3ABNDT%5F50K%2FNTDB%5F50K,PINGO%5F0%3ABNDT%5F50K%2FNTDB%5F50K%2CENTREE%5FDE%5FCAVERNE%5F0%3ABNDT%5F50K%2FNTDB%5F50K,CONTOUR%5FLABELS%5F1%3ABNDT%5F50K%2FNTDB%5F50K,TOPONYME%5F0%3ABNDT%5F50K%2FNTDB%5F50K,INDEX%5FANNOTAT%5FTHICK%5F50K&REQUEST=GetMap&SERVICE=WMS&FORMAT=image%2Fgif&STYLES=,,,,,,,,,,,,,,&HEIGHT=666

Note how the original blocks of parameter values are encoded, but the commas
added by the merging process isn't?  Same source of problems, though this one
isn't a big deal.

Easy way to test would be to replace:

                sprintf(pszBuf, "%s,%s", value1, value2);

with:

                sprintf(pszBuf, "%s%2C%s", value1, value2);

?

[mapserver-bot]

Author: assefa
Date: 2006/03/16 - 17:21

JF,

 I was checking this a bit this morning :  

  - the request you put in in comment #3  has 15 layers in the LAYERS parameters
with 15 Styles defined in the STYLES parameter (14 commas). This is a valid
request according to the wms specs (1.1.1 section 7.2.3.4)  (If all layers are
to be shown using the default style, either the form "STYLES=" or "STYLES=,,,")

 - Does the server chockes on the request ? Which version of Maperver server are
you using (I am assuming cvs)

[mapserver-bot]

Author: assefa
Date: 2006/03/16 - 17:22

JF,

 I was checking this a bit this morning :  

  - the request you put in in comment #3  has 15 layers in the LAYERS parameters
with 15 Styles defined in the STYLES parameter (14 commas). This is a valid
request according to the wms specs (1.1.1 section 7.2.3.4)  (If all layers are
to be shown using the default style, either the form "STYLES=" or "STYLES=,,,")

 - Does the server return an error message on the request ? Which version of
Maperver server are you using (I am assuming cvs)

[mapserver-bot]

Author: jdoyon@nrcan.gc.ca
Date: 2006/03/16 - 17:44

I am using 4.8.1 as the client, I could check, but the server is most likely the
same.

Actually, there are far more than 15 layers.  Note how I mentionned the encoding.

If you count the '%2C' AND the actual ',', you should find many many more layers.

15 seems to be the number of requests that were "merged" if you will, or
something like that, which is why you have encoded lists of layers, seperated by
unencoded commas.

[mapserver-bot]

Author: assefa
Date: 2006/03/16 - 19:15

JF,

 Sorry I missed the encoding part. The "problem" comes from the fact that the
layer name that are used conatain comma into them. Example one of the wms client
layer is named 
 "wms_name"          
"SENTIER_1:BNDT_50K/NTDB_50K,ROUTE_ACCES_LIMIT_1:BNDT_50K/NTDB_50K"

 I think that the server would have given you an error (regradless of the styles
parameter). A wms name with a coma would generate a big warning when you do a
capabilities because of potential problems.
 Is it possible that the names need to really have a comma in them ?

[mapserver-bot]

Author: jdoyon@nrcan.gc.ca
Date: 2006/03/16 - 19:56

According to the WMS Client How-To:

"wms_name" metadata - comma-separated list of layers to be fetched from the
remote WMS server. This value is used to set the LAYERS and QUERY_LAYERS WMS URL
parameters.

The layer names don't have comma's ... it's a LIST of comma seperated layer names.

[mapserver-bot]

Author: assefa
Date: 2006/03/16 - 21:51

JF you are right about the names. I think I got a problem :)

 What I am suggesting as a fix is that the defaut styles string generated would
parse the layer name to see if there are several layers and generate commas for
each layer. I am not sure that I see other solutions. I will aso encode the commas.
Of course if the user specifies a style using wms_style metadata , he is
responsible of providing comma separated styles for each layer.
Is this accepatable ?

[mapserver-bot]

Author: dmorissette
Date: 2006/03/16 - 22:11

You should check the spec before encoding all commas, I seem to remember some
rules in the WMS spec about commas not always being encoded in WMS params
depending on context.

[mapserver-bot]

Author: jdoyon@nrcan.gc.ca
Date: 2006/03/16 - 23:29

Sure that solution sounds acceptable.  Would it not be simpler to remove
unnecessary commas altogether though?  That's what my patch attempted to do ...
don't join empty strings together :)

As for the encoding, I don't to it, MapServer does that.  It encodes request
parameters before WMS request merging.

The unencoded commas in the STYLES= are a product of WMS request merging.

[mapserver-bot]

Author: assefa
Date: 2006/03/19 - 23:22

We have to generate empty styles (with commas) since some layers may actually
have a style defined. In that case you need empty styles for those layers that
do not have a style combined with styles for those that have a defined style.

[mapserver-bot]

Author: jdoyon@nrcan.gc.ca
Date: 2006/03/20 - 22:51

How about simply using "default" instead of an empty string?

[mapserver-bot]

Author: tomkralidis
Date: 2008/06/04 - 04:26
Any update on this issue? Note that #1088 has since been implemented, which may help.

[jratike80]

I made a short test with Mapserver 7.0. if I have two layers in &LAYERS=layer1,layer2 then the generated GetMap is having &STYLES=,
So two layers, two empty styles. Closing as fixed.