Regression in 5.6.4 regarding handling of NullShapes

[woodbri]

I'm not sure when this regression crept in because I'm converting a lot of older apps over to use 5.6.4. But on 5.2.2 I could draw a layer with NullShape items in a shapefile without a problem and on 5.6.4 it dies with a general error message.

msDrawMap(): Image handling error. Failed to draw layer named 'Linear Water'. msDrawShape(): General error message. Only polygon or line shapes can be drawn using a line layer definition.

In general, since mapserver is designed for drawing data and not validating it or being a GIS system in general. I think we should just step over these records. If we want to be more helpful AND the user has DEBUG truned on than it would be helpful to say what file and what record is a NullShape.

I have a huge tileindex that I converted to script like: # shpdump ./afghanistan_p/major_roads/aflw | grep Shape: with 128 files and anyones guess on the number of records.

woodbri@carto:/u/data/leaddog$ dbfdump tidx-lw.dbf > bbb
woodbri@carto:/u/data/leaddog$ vi bbb
woodbri@carto:/u/data/leaddog$ sh bbb | grep -v Arc
Shape:0 (NullShape)  nVertices=0, nParts=0
Shape:1 (NullShape)  nVertices=0, nParts=0

Here is a patch that seems to fix this:

woodbri@carto:/u/software/mapserver-5.6.4$ diff -Naur mapdraw.c mapdraw.c-orig
--- mapdraw.c   2010-07-10 11:53:17.000000000 -0400
+++ mapdraw.c-orig      2010-07-10 11:57:27.000000000 -0400
@@ -964,7 +964,7 @@
   while((status = msLayerNextShape(layer, &shape)) == MS_SUCCESS) {

       shape.classindex = msShapeGetClass(layer, &shape, map->scaledenom, classgroup, nclasses);
-    if((shape.type == MS_SHAPE_NULL) || (shape.classindex == -1) || (layer->class[shape.classindex]->status == MS_OFF)) {
+    if((shape.classindex == -1) || (layer->class[shape.classindex]->status == MS_OFF)) {
        msFreeShape(&shape);
        continue;
     }

[dmorissette]

I can't find a change between 5.6.0 and 5.6.4 that would explain this.

Are you getting this from Mapscript (and which one), the mapserv CGI, or both?

Also, can you please try turning on debug level 5 for your mapfile and layer and see if that error message ended up in the logs with v5.2.2. I am wondering if it could be that something changed in the way this error is propagated, and maybe in 5.2.2. it was just lost and in 5.6.x we do a better job of propagating it.

[woodbri]

I'm just using mapserver CGI. Well regardless, I think we should just step over null shape objects and at most report the file and record number if DEBUG is on.

My patch above seems to step of the null shapes if that is enough. Reporting is not required but might be a nice to have.

[dmorissette]

I have no problems doing this in trunk, but I am hesitant with what looks like simple changes but can potentially have side-effects that could break a bugfix release, like what happened with #3173 vs 5.6.4.

That's why I would rather understand when this difference was introduced and why before we attempt to fix it in a stable branch... since what you came across is obviously an unexpected side-effect of another change.

What do you think SteveL?

[sdlime]

I agree with Dan. The NULL shape piece needs to be fully understood and I would not want to add this to the 5.6.5 release unless we could link it to a change elsewhere.

Steve

[aboudreault]

Hi Steve, I've looked deeper and I think you'll need to provide a small test case. I've created a simple shapefile that contains a null shape and tried to render it. It renders properly skipping the null shape. I've also put a few debug outputs in the code and I can see that the null shape is skipped in the msShapeFileLayerNextShape function in mapshape.c (line 2530):

...
/* skip NULL shapes (apparently valid for shapefiles, at least ArcView doesn't care) */
if(shape->type == MS_SHAPE_NULL) return(msLayerNextShape(layer, shape));
...

I'm going to attach my test case.

[aboudreault]

shapefile with null shape

[aboudreault]

to get the example to work: simple remove the FONTSET line in the test.map file, and try it in a browser: ​http://localhost:8080/cgi-bin/mapserv?map=test.map&mode=map&layer=test

[woodbri]

Arrrgh, I will have to locate the two files in my data set that demonstrate this problem which I will do, but it will have to wait for a week as I'm packing for a trip and leave 8am in the morning.

I'm using a tileindex if that makes a difference. I would also ask why does my patch work for my data? Obviously your test case does not go through the same code path as I'm hitting. So if it is not because of the tileindex using a different code path, I will try to isolate a sample that reproduces the problem.

[dmorissette]

I modified Alan's test case to use a tileindex and we still do get a map and no error message. I will attach my modified test case to this ticket.

I think we'll have to go ahead with 5.6.5 without this one since we won't be able to reproduce it until you are back in a week, and as explained earlier I would like to understand where this error/change comes from before patching and risk creating new side-effects.

[dmorissette]

Modified test case using a tile index

[woodbri]

I did a scan of all my files were I was having a problem and found the following. It might be that the shapefile handling is checking if nVertices=0 and return its type as NULL_SHAPE.

Not that I know what files might be problematic, I will try and extract a single record that reproduces the problem. If this is not the problem then I will have to add some debug output to mapserver to try and identify there it is coming from.

I wrote a perl script to run something like the following against all my files:

shpdump test.shp | grep -i -E -e 'null|nVertices=0'

Srcipt results are:

Checking 2594 files ...
Shape:45896 (Arc)  nVertices=0, nParts=0
Shape:4045 (Arc)  nVertices=0, nParts=0
Shape:3 (Polygon)  nVertices=0, nParts=0
Shape:9594 (Arc)  nVertices=0, nParts=0
Shape:62429 (Arc)  nVertices=0, nParts=0
Shape:0 (NullShape)  nVertices=0, nParts=0
Shape:271 (Polygon)  nVertices=0, nParts=0
Shape:6477 (Arc)  nVertices=0, nParts=0
Shape:641 (Polygon)  nVertices=0, nParts=0
Shape:11906 (Arc)  nVertices=0, nParts=0
Shape:5631 (Arc)  nVertices=0, nParts=0