Opened 22 years ago

Closed 21 years ago

#186 closed defect (fixed)

[PHP MapScript] Bug in project() method of all geometry objects

Reported by: dmorissette Owned by: dmorissette
Priority: highest Milestone:
Component: MapScript-PHP Version: 4.0
Severity: normal Keywords:
Cc: pspencer@…

Description

The shapeObj, rectObj, pointObj, etc. all have a project() method that behaves 
in a similar way, and each contain a similar bug:

The project() method works on the object itself, not on a copy... this is OK 
since it's the intended behavior in libmap.a ... however the PHP wrappers 
initialize a new PHP object and return that new object to the caller instead of 
resetting the current object's properties in the PHP wrapper as one would expect 
(and returning a status flag).

The problem with the new object is that it contains a reference to the same 
internal C data structure as the original object, so we end up with two PHP 
object wrappers referring to the same C data structure... after the first object 
is deleted then the second contains an invalid reference and can cause a 
crash... the following code sequence will reproduce the crash:

  // Create a rectobj to reproject
  $rect = ms_newRectObj();
  $rect->setExtent(....);
  $proj1 = ns_newProjectionObj("...");
  $proj2 = ns_newProjectionObj("...");
  // Reproject the rest and overwrite the value of $rect... this is where the 
  // problem happens since overwriting the value of $rect deleted the original
  // object.
  $rect = $rect->project($proj1, $proj2);
  // now to produce a crash, try to force accessing the internal data structure
  // with a call suuch as setExtent() on the rectObj:
  $rect->setExtent(...);  // Pouf!  Crash because the internal rectObj was
                          // deleted by the assignement to $rect above.

The solution:

I suggest modifying all the project() methods to behave like the SWIG 
equivalents:
- Return a MS_SUCCESS/MS_FAILURE status instead of a new object instance
- Make sure all members of the PHP wrapper are updated before the function
  returns (i.e. $point->x, y, m,  $rect->minx, miny, maxx, maxy,  etc.)

This should be done in 3.7 only as it breaks compatibility with previous code 
that expects to receive a new instance of an object as the return value... but 
there will be so many incompatibilities between 3.6 and 3.7 anyway that as long 
as we document this it shouldn't be too bad.

Change History (3)

comment:1 by dmorissette, 21 years ago

Cc: spencer@… added

comment:2 by dmorissette, 21 years ago

Priority: highhighest

comment:3 by dmorissette, 21 years ago

Resolution: fixed
Status: newclosed
Fixed in 3.7.  
As described above, the methods now return MS_SUCCESS/MS_FAILURE and update the 
PHP object itself instead of returning a copy.
Note: See TracTickets for help on using tickets.