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)
Change History (13)
follow-up: 2 comment:1 by , 16 years ago
Cc: | added |
---|
follow-up: 4 comment:2 by , 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 , 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.
follow-up: 9 comment:4 by , 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 , 16 years ago
Cc: | 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 , 16 years ago
Stephan, can you make some sample data (map+shp would be perfect) available so that we can reproduce the error?
by , 16 years ago
Attachment: | ms-5.2.0-tests-issue2803.tar.gz added |
---|
testcase for issue 2803 with php/mapscript
comment:7 by , 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 , 15 years ago
Cc: | added |
---|
by , 15 years ago
Attachment: | 2803_exit_with_error.patch added |
---|
comment:9 by , 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.
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