segmentation violation when layer item indexes are used and initValues hasn't been called

[karl-ms@…]

If initValues is needed but hasn't been called or hasn't been called with enough values, a segmentation 
violation occurs.

For example, say I have defined a LABELANGLEITEM in a layer but haven't called initValues on every 
shape in that layer that has a label, shape->values will be NULL, causing a pointer dereference at line 
1621 of mapdraw.c:

1621              if(layer->labelangleitemindex != -1) label.angle = atof(shape->values
[layer>labelangleitemindex]);

Program terminated with signal 11, Segmentation fault.

(gdb) p layer->labelangleitemindex
$2 = 0
(gdb) p shape->values
$3 = (char **) 0x0

Suggest this be detected and returned as an error rather than generating a coredump.  One possibility 
is to check to see if layer->numitems is nonzero and, if so, at least verify that shape->values points to 
something.  Otherwise it's a coredump anytime you're using an index variable in a shape where you 
haven't called initValues with the right number as an argument.

Thanks for mapserver -- it's great!

Karl Lehenbauer
(the Tcl guy)

[sdlime]

Karl: How are you triggering the error? Via MapScript, the CGI or ???

Steve

[karl-ms@…]

I'm triggering it via mapscript.  Code that worked under 4.8 causes the coredump under 4.10 beta 
because, I believe, initValues now needs to be called and it didn't previously.  If it hasn't been called, this 
causes the null pointer dereference attempt.

[szekerest]

Before 4.10 creating a new shapeObj has also created 4 predefinied values
causing possible memory leaks in some conditions. This behaviour was changed and
now the user should call initValues explicitly to allocate memory for the values
(for the inline shapeObjs only). When the layer's data is coming from a data
provider the creation of the values are handled internally so you must not use
initValues at all.

I wonder how you are using labelangleitemindex now. In general the ...itemindex
is handled internally. 
In maplayer.c 551
if(layer->labelangleitem) layer->labelangleitemindex = string2list(layer->items,
&(layer->numitems), layer->labelangleitem);

The user should only use the corresponding ...item properties.

However for the SWIG mapscript I am not sure if the interface will set
...itemindex when setting ...item properly. So a bug report will be needed on
this issue.


Tamas Szekeres 

[szekerest]

labelangleitemindex should be set internally by msLayerWhichItems which is
called before calling msDrawShape. I consider this kind of problem may occur
when labelangleitemindex is set externally and the drawing of the annotation is
suppressed  for that layer (by using labelmaxscale for example).

[sdlime]

That property should be immutable then. There is no need for an application to 
modify it directly. I thought that was the case but must have been mistaken.

Steve

[szekerest]

I'd prefer hiding labelangleitemindex (along with labelitemindex and
labelsizeitemindex) entirely.

Tamas

[szekerest]

I've just committed the changes to CVS-HEAD so as to be ready for 4.10.0

Tamas

[hobu]

Looks like this can be closed. Re-open if necessary...