Opened 18 years ago
Last modified 15 years ago
#1793 new defect
POSITION (null) after mapfile write
Reported by: | Owned by: | dmorissette | |
---|---|---|---|
Priority: | high | Milestone: | 6.0 release |
Component: | MapServer C Library | Version: | unspecified |
Severity: | normal | Keywords: | |
Cc: |
Description (last modified by )
After upgrading to the CVS version (4.9?) of mapserver (30th May 2006) - I have discovered a bug in the $map->save() function. In order to pass the map object between page reloads we save it out to a temporary file and then reintialise it again on the next page reload. Using the 4.8.3 mapscript this seemed to work fine - however, in the CVS version it seems to put a POSITION (null) in the label section (even when it is set using $labelObj->set("position", "CC");)
This causes the map to not load and it crashes out with the error:
<b>Warning</b>: Failed to open map file /srv/www/htdocs/ark/mapserver/tmp/5c7975a60654928b0921582c87969c4c.map in <b>/srv/www/htdocs/ark/php/map/getmap.php</b> on line <b>62</b><br />
Attachments (1)
Change History (11)
by , 18 years ago
Attachment: | 5c7975a60654928b0921582c87969c4c.map added |
---|
comment:1 by , 18 years ago
Cc: | added |
---|---|
Component: | MapScript-PHP → MapServer C Library |
Owner: | changed from | to
Actualy this is a mapfile.c (msSaveMap()) issue, not specific to PHP MapScript. Switching component.
comment:2 by , 18 years ago
attachments.mimetype: | application/octet-stream → text/plain |
---|
comment:3 by , 18 years ago
Oh wait, this may partly be a PHP issue after all... you pass "CC" as the value to set the position. You should use the constant MS_CC: $labelObj->set("position", MS_CC) Can you please try that and confirm that "POSITION CC" is written to your mapfile after that. There remains an issue in msSaveMap() should not write "POSITION (null)" when POSITION is not set properly.
comment:4 by , 18 years ago
Daniel, Yep sorry that was a mistake on my part - when I pass MS_CC it works fine. However, I will have times when thats not set - so hopefully the mapfile.c bug will get sorted out - thanks for your help!
comment:5 by , 18 years ago
I had a look in the code and I'm not sure if we should call that a mapfile.c bug, a PHP MapScript bug, or a generic MapScript bug. I am documenting it here since similar issues may affect other MapScripts. What happens is that when you call $labelObj->set("position", "CC"), the PHP MapScript implementation is very forgiving and attempts to convert the string "CC" to an integer... which turns out to be 0 in this case. Then it sets label->position in the C structure to 0. Then in mapfile.c we call: fprintf(stream, " %sPOSITION %s\n", tab, msPositionsText[label->position - MS_UL]); Since label->position is zero, the array index is negative and pointing to an invalid memory area. Having label->position (or any other struct member) set to zero by defautl is usually not an issue since 0 is almost always a valid enum value (POSITION is an exception on that front). We could modify PHP MapScript to be less forgiving and complain right away if a string is passed for an integer parameter like this. However there are cases where a string may contain a valid value (e.g. "123") and adding type validation at this late point in PHP MapScript's life may cause grief to lots of existing code. Plus that would not prevent users from passing arbitrary integer values ot the set() method and still cause the same crash. Also I fear that other MapScripts may suffer from the same level of forgiveness and allow invalid values to be set too. We could fix the mapfile.c code to check that label->position contains a valid value before accessing the msPositionText[] array. This would be an easy fix, but there may be many places that do similar things without any validation that should be fixed too. What do you think is best Steve?
comment:6 by , 18 years ago
Ugh... Ideally we'd do both. Seems like PHP/MapScript should be accepting a integer there (or symbol, MS_CC) and complain otherwise. SWIG enforces the underlying type (position is an int) so that would be consistent with how it behaves. (I need to test that theory.) That said, as Dan mentions, one could always set a value that is out of range so the mapfile writer should make sure the position is in range before attempting to write the text equivalent. We're probably woefully inadequate with those type of checks. In general I like the idea of catching these types of issues as early as possible (bad values may cause problems elsewhere), but perhaps the latter fix to the writer gives us ultimately more protection. Steve
comment:7 by , 16 years ago
Description: | modified (diff) |
---|---|
Milestone: | → 5.2 release |
Owner: | changed from | to
comment:8 by , 16 years ago
Description: | modified (diff) |
---|
comment:9 by , 16 years ago
Milestone: | 5.2 release → 5.4 release |
---|
Moving to 5.4... Progress has been made so perhaps we should kill this bug and create two that are more meaningful. One for data typing in PHP/MapScript and another for bounds checking in the write...() functions in mapfile.c.
Steve
comment:10 by , 15 years ago
Milestone: | 5.4 release → 6.0 release |
---|
sample mapfile as created using $map->save()