MapServer / MapServer-import

3 stars 2 forks source link

msUpdateClassFromString segfaults when class is not in a layer #2220

Closed tbonfort closed 12 years ago

tbonfort commented 12 years ago

Reporter: sgillies Date: 2007/08/09 - 20:34 Since version 4.4 (maybe even earlier), we've been allowing users to create independent classObjs like

  >>> co = mapscript.classObj()

My current implementation of mapscript.fromstring (for RFC 31) leverages that to create new classObjs from a string:

  >>> co = mapscript.classObj()
  >>> co.updateFromString(data)

Currently, msUpdateClassFromString assumes that a classObj has a parent layerObj and we get a segfault. From running the doctest:

Trying:
    co = mapscript.fromstring("""CLASS END""")
Expecting nothing

Program received signal SIGSEGV, Segmentation fault.
---Type <return> to continue, or q <return> to quit---
[Switching to Thread -1210665296 (LWP 23390)]
0xb797394a in msUpdateClassFromString (class=0x825c0b8, 
    string=0xb7a5a304 "CLASS END", url_string=0) at mapfile.c:2277
2277      if(loadClass(class, class->layer->map, class->layer) == -1) {

class->layer is NULL at that point. Solution: do not dereference class->layer if it is NULL.

tbonfort commented 12 years ago

Author: sdlime Date: 2007/08/09 - 20:44 Sean: Should msUpdateClassFromString() just return silently then if !class->layer? I don't see an alternative although I should see if we could plod through loading sub-objects with a NULL map or layer. Basically the loadObject functions would need to be more bullet proof...

Steve

tbonfort commented 12 years ago

Author: sgillies Date: 2007/08/09 - 20:57 The only use loadClass makes of the map is as a container for the templatepattern. I suggest we change the signature of loadClass from

int loadClass(classObj *class, mapObj *map, layerObj *layer)

to

int loadClass(classObj *class, char *templatepattern, layerObj *layer)

I also just noticed that loadClass never even uses the layer parameter. That should be eliminated while we're at it. loadClass is only called 3 times in mapfile.c so it's a small fix.

tbonfort commented 12 years ago

Author: sdlime Date: 2007/08/09 - 21:06 Sounds ok, could you make the change?

Steve

tbonfort commented 12 years ago

Author: sgillies Date: 2007/08/09 - 21:28 Committed to 8a7b82582d750f51e70040f0752db9f2e71faf5e (r6542). Mapscript tests look ok, no related failures. Try it out?

tbonfort commented 12 years ago

Author: sdlime Date: 2007/08/31 - 05:41 I believe this one is good to go... Thanks Sean!

Steve