Opened 16 years ago

Closed 15 years ago

#2803 closed defect (fixed)

mapserver will crash when rendering a query map in HILITE mode and there are is no STYLE defined

Reported by: unicoletti Owned by: sdlime
Priority: normal Milestone: 5.2.1 release
Component: MapServer C Library Version: svn-trunk (development)
Severity: normal Keywords:
Cc: dmorissette, sdlime, sholl, unicoletti

Description

mapserver (and mapscript) will crash when the following conditions are both true:

  • rendering a query map in HILITE mode
  • tha map has at least one queried layer with a class without any STYLE element

the crash occurs in mapdraw.c:msDrawQueryLayer because the code dereferences a pointer without checking first. The same map used to work in versions < 5 and the bug was introduced by the refactoring of layers, classes and styles from arrays of structs to arrays of pointers.

Proposed fix: a new style should be allocated on the fly on pushed on the array. This however must be done carefully so that the reference counting code is notified of the new element.

This issue was brought up in mapserver-users mailing list, thread 'MS 5.2.0 and QUERYMAP Hilite', reference to gmane will be made when available.

Attachments (3)

2803_proposed.patch (3.0 KB ) - added by unicoletti 16 years ago.
Proposed patch: fixes the error for me
ms-5.2.0-tests-issue2803.tar.gz (20.4 KB ) - added by sholl 16 years ago.
testcase for issue 2803 with php/mapscript
2803_exit_with_error.patch (802 bytes ) - added by unicoletti 15 years ago.

Download all attachments as: .zip

Change History (13)

comment:1 by dmorissette, 16 years ago

Cc: dmorissette sdlime added

If a class has no style, then that presumably means that the objects from that class should not be drawn, right?

In this case, would the right fix not simply be to do nothing for classes that have no style? i.e. simply skip any class with no style in msDrawQueryLayer()... I find the idea of allocating an empty style to work around a seg fault a bit clunky

in reply to:  1 ; comment:2 by unicoletti, 16 years ago

Replying to dmorissette:

If a class has no style, then that presumably means that the objects from that class should not be drawn, right?

wrong, because it could have the color/outlinecolor defined directly into the class (a style element is not required).

In this case, would the right fix not simply be to do nothing for classes that have no style? i.e. simply skip any class with no style in msDrawQueryLayer()... I find the idea of allocating an empty style to work around a seg fault a bit clunky

nope, we have to draw the object for the reason given above OR change the map file syntax to disallow classes with no style (how cool is that, btw?). The latter probably requires an RFC or at least a discussion on -dev.

comment:3 by dmorissette, 16 years ago

Unless I'm mistaken, the color/outlinecolor defined directly in the class are supported only at the parser level. Setting color/outlinecolor directly in the class in the mapfile does result in a style being allocated by the parser and all... so this is not a case we need to be worried about here.

by unicoletti, 16 years ago

Attachment: 2803_proposed.patch added

Proposed patch: fixes the error for me

in reply to:  2 ; comment:4 by sdlime, 16 years ago

Replying to unicoletti:

Replying to dmorissette:

If a class has no style, then that presumably means that the objects from that class should not be drawn, right?

wrong, because it could have the color/outlinecolor defined directly into the class (a style element is not required).

Actually this is not wrong. COLOR and OUTLINECOLOR at the class level are simply shortcuts for modifying the first style.

In this case, would the right fix not simply be to do nothing for classes that have no style? i.e. simply skip any class with no style in msDrawQueryLayer()... I find the idea of allocating an empty style to work around a seg fault a bit clunky

nope, we have to draw the object for the reason given above OR change the map file syntax to disallow classes with no style (how cool is that, btw?). The latter probably requires an RFC or at least a discussion on -dev.

Personally I would leave well enough alone and just add code to throw an error- "Attempting to draw a feature with no style" rather than tring to figure out anything else.

Steve

comment:5 by sholl, 16 years ago

Cc: sholl added

Thanks for the fast patch. I tried it against the 5.2.0-relase-sources. It applied cleanly, but the result is identically. MS still segfaults on querying layers without style-blocks in classes.

The original thread to this issue can be found here: http://lists.osgeo.org/pipermail/mapserver-users/2008-November/058380.html

comment:6 by unicoletti, 16 years ago

Stephan, can you make some sample data (map+shp would be perfect) available so that we can reproduce the error?

by sholl, 16 years ago

testcase for issue 2803 with php/mapscript

comment:7 by sholl, 16 years ago

Umberto,

I have added a simple testcase including a shape, mapfile and simple php-script for testing. Mapserver crashes with this testcase, also when applying the patch '2803_proposed.patch'.

comment:8 by unicoletti, 15 years ago

Cc: unicoletti added

by unicoletti, 15 years ago

Attachment: 2803_exit_with_error.patch added

in reply to:  4 comment:9 by unicoletti, 15 years ago

Replying to sdlime:

Personally I would leave well enough alone and just add code to throw an error- "Attempting to draw a feature with no style" rather than tring to figure out anything else.

I eventually found time to look through this more carefully and I think you are right. I have attached a patch to this ticket that implements error checking. Let me know what you think about it so that you or I can commit the change and close the ticket.

Sample output (in python):

Traceback (most recent call last):

File "./testscript.py", line 11, in <module>

image=map.drawQuery()

File "/usr/lib/python2.5/site-packages/mapscript.py", line 874, in drawQuery

def drawQuery(*args): return _mapscript.mapObj_drawQuery(*args)

_mapscript.MapServerError: msDrawMap(): Image handling error. Failed to draw layer named 'gewaesser'. msDrawQueryLayer(): General error message. Don't know how to draw class of layer gewaesser without a style definition.

comment:10 by unicoletti, 15 years ago

Resolution: fixed
Status: newclosed

Patch applied in r8533

Note: See TracTickets for help on using tickets.