ST_Dump kills backend when fed CIRCULAR STRING

[robe]

What steps will reproduce the problem? 1. SELECT ST_Dump(ST_GeomFromEWKT('CIRCULARSTRING(220268 150415 1,220227 150505 2,220227 150406 3)'));

2. Crash 3. Does the same on both 1.3.4SVN and 1.3.3

[mcayland]

Hmmm I can't recreate this here on 64-bit Debian. I see this:

postgis13=# SELECT ST_Dump(ST_GeomFromEWKT('CIRCULARSTRING(220268 150415 1,220227 150505 2,220227 150406 3)'));

st_dump

---

(0 rows)

Which platform is this? Maybe the problem only shows on a particular platform or 32/64 bit architecture?

ATB,

Mark.

[robe]

32-Bit OpenSUSE and 32-bit windows xp. Though that is the wrong answer anyway. It should throw a not supported error or return {0}{CIRCULARSTRING(....)}

[robe]

Ah slight correction it should return empty path {} and {CIRCULARSTRING(..)}. Just retested and get the same crash. server closed the connection unexpectedly

This probably means the server terminated abnormally before or while processing the request.

The connection to the server was lost. Attempting reset: Failed.

Full version info:

ver_pg_bd

---

PostgreSQL 8.2.6 on i686-pc-linux-gnu, compiled by GCC gcc (GCC) 4.2.1 (SUSE Linux)

POSTGIS='1.3.4SVN' GEOS='3.1.0-CAPI-1.5.0' PROJ='Rel. 4.6.0, 21 Dec 2007' USE_STATS 2008-11-05 12:43:25

Same test on windows XP 'PostgreSQL 8.3.1, compiled by Visual C++ build 1400 POSTGIS='1.3.3' GEOS='3.0.0-CAPI-1.4.1' PROJ='Rel. 4.6.0, 21 Dec 2007' USE_STATS 2008-04-15 18:49:24'

Ah it completely kills PostgreSQL on windows - have to restart the service afterward.

[mcayland]

Hmmm it seems that this is mainly triggered on 32-bit platforms. Since I don't have access to my Win32 virtual or a spare 32-bit machine here, does anyone else want to have a go at this?

ATB,

Mark.

[robe]

Mark,

What about the case in 64-bit where it returns 0 rows. It should return 1 row in this example or throw an error that this type is not supported. I actually haven't looked at ST_Dump code at all, but would be willing to take a stab at it if no one else more experienced has time to do so.

[robe]

Mark,

I took a quick glance at the Dump code in lwgeom_dump.c. The first glaring problem I see is that it assumes that all geometry types (from 4 on ) #define MULTIPOINTTYPE 4 are multis or collections. All the curved types are in the 8+ range. So I am guessing it is trying to break up a circular string which can't be broken up and runs into a fault as as result. This we should at least change to not hard code the assumption that >= MULTITYPE are MULTIs or collections. It would be nice if we can come up with a more elegant way of checking this rather than the number range which is prone to change.

Anyrate I'll try to change this to a case range later and see how that fairs. I'm sure there are more complications further down (the code later on looks a bit scarier to digest).

[robe]

Hey Mark - I got it to work by making the fixes I mentioned above (3 locations as far as I can tell). I'm going to test out with multicurves first and then submit a patch to you if all looks good. I would prefer you look over it when I am done instead of me just committing it since we are about to release.

Though I stuck with itemizing because I don't know the code base well enough to

threaten doing anything more sophisticated especially so close to release.

Thanks, Regina

[mcayland]

Regina,

Your analysis was spot on. Since the same check is required in 3 separate places, it made sense to break it out into a separate function rather than keep repeating the code. I've committed the fix to both 1.3 branch and SVN trunk - please can you test at your end?

Many thanks,

Mark.

[robe]

Mark,

Thanks - I will next chance I get. By the way I saw your commit and that's the kind of elegance I was looking for :).

[robe]

Okay just tested this and seems to work fine now.