Opened 15 years ago

Closed 14 years ago

#2896 closed defect (fixed)

Division by zero problem in mapserver

Reported by: tamas Owned by: tamas
Priority: normal Milestone: 5.6 release
Component: MapServer C Library Version: unspecified
Severity: normal Keywords:
Cc: tbonfort, sdlime, dmorissette, havatv

Description

I find in many places in the code the following fragment:

scale = size/symbol->sizey;

without actually checking the value of symbol->sizey which may be zero eventually. This causes me unexpected problems, like for example a premature termination of the application during the pdf output.

I suggest to fix this problem by setting symbol->sizex and symbol->sizey to 1 instead of 0 by default

Attachments (1)

test.map (436 bytes ) - added by sdlime 15 years ago.
Mapfile that shows the problem depending if SYMBOLSCALE line is active or not.

Download all attachments as: .zip

Change History (9)

comment:1 by tamas, 15 years ago

Owner: changed from sdlime to tamas

comment:2 by tamas, 15 years ago

Resolution: fixed
Status: newclosed

Fixed in r8574

comment:3 by sdlime, 15 years ago

Resolution: fixed
Status: closedreopened

I've found side effect of this that needs to be addressed. If doing solid polygon fills with symbol scaling (e.g. SYMBOLSCALE) for some reason we're getting scaling with the default symbol (symbol 0) where we had not before. This may be a product of supporting doubles for SIZE as well. My guess is that a SIZE of 0 triggers certain behavior that values near 0 or below 1 don't. I've not looked at the code but it appears with both GD and AGG.

The problem is that I think many folks use solid fills with scalable labels and the result will be perplexing. The workaround is to set "MINSIZE 1" but it would be better to fix in the source rather than forcing mapfile changes.

Steve

in reply to:  3 comment:4 by tamas, 15 years ago

Steve,

Do you have an use case (mapfile/script) this issue could be investigated with?

Replying to sdlime:

I've found side effect of this that needs to be addressed. If doing solid polygon fills with symbol scaling (e.g. SYMBOLSCALE) for some reason we're getting scaling with the default symbol (symbol 0) where we had not before. This may be a product of supporting doubles for SIZE as well. My guess is that a SIZE of 0 triggers certain behavior that values near 0 or below 1 don't. I've not looked at the code but it appears with both GD and AGG.

The problem is that I think many folks use solid fills with scalable labels and the result will be perplexing. The workaround is to set "MINSIZE 1" but it would be better to fix in the source rather than forcing mapfile changes.

Steve

comment:5 by sdlime, 15 years ago

I should know how to file a bug shouldn't I. My use case here is too complex for general debugging. Let me whip up a stand-alone version...

Steve

comment:6 by sdlime, 15 years ago

Cc: tbonfort added
Resolution: fixed
Status: reopenedclosed

The solution was (in shade symbol drawing code for GD and AGG) to bail if a size was too small (< 1) only if style->symbol was not 0 (the default). I really didn't try to follow the code all the way through. It's not obvious to me why changing the default would have this side effect. We may run into others. It's fixed for now. I'll upload a test mapfile that demonstrates the error if using the code prior to r8614. Commenting out the SYMBOLSCALE line will cause the feature (a big red box) to appear or disappear.

Steve

by sdlime, 15 years ago

Attachment: test.map added

Mapfile that shows the problem depending if SYMBOLSCALE line is active or not.

comment:7 by tbonfort, 14 years ago

Cc: sdlime dmorissette havatv added
Resolution: fixed
Status: closedreopened

setting the default size to 1 makes the mapserver code incorrectly calculate the size of a vector symbol where all points have coordinates <1.

offending code is in mapsymbol.c, around line 341:

s->sizex = MS_MAX(s->sizex, s->points[s->numpoints].x);
s->sizey = MS_MAX(s->sizey, s->points[s->numpoints].y);

comment:8 by tbonfort, 14 years ago

Milestone: 5.6 release
Resolution: fixed
Status: reopenedclosed

old behavior "reverted" in r9516

the size calculation code should really be rethought to aknowledge negative values in vector points.

Note: See TracTickets for help on using tickets.