Opened 16 years ago

Closed 16 years ago

#2674 closed defect (fixed)

Creation of new shape object from scratch within php_mapscript caused an error

Reported by: nsavard Owned by: dmorissette
Priority: normal Milestone: 5.2 release
Component: MapScript-PHP Version: svn-trunk (development)
Severity: normal Keywords:
Cc: assefa, sdlime, rouault, warmerdam

Description (last modified by dmorissette)

I created a new shape file object within php_mapscript and got the error below. I tested with MapServer 5.2.0 beta2, beta 3 and trunk on Ubuntu. It doesn't happen with 5.0.2 release. I pasted below the script used for the test. 

[02-Jul-2008 15:27:21] PHP Warning:  [MapServer Error]: msNewSHPFile(): (ttt)
 in /home/nsavard/fgs-ms5.2.0.beta3/www/htdocs/createshp.phtml on line 9
[02-Jul-2008 15:27:21] PHP Warning:  [MapServer Error]: msSHPOpen(): Corrupted .shp file : nRecords = -12.
 in /home/nsavard/fgs-ms5.2.0.beta3/www/htdocs/createshp.phtml on line 9
[02-Jul-2008 15:27:21] PHP Fatal error:  Failed to open shapefile ttt in /home/nsavard/fgs-ms5.2.0.beta3/www/htdocs/createshp.phtml on line 9

and the script is: 

<html>
<body>
<?php

dl("php_mapscript.so");

$shpFname = "ttt";
$shpFile = ms_newShapeFileObj( $shpFname, MS_SHP_POINT );

?>
</body>
</html>

Change History (4)

comment:1 by dmorissette, 16 years ago

Cc: assefa sdlime added
Description: modified (diff)
Owner: changed from mapserverbugs to dmorissette
Status: new → assigned

I am able to reproduce this here. Working on it now.

comment:2 by dmorissette, 16 years ago

Cc: rouault warmerdam added

Stepping in the code, the error is produced at line 299 of mapshape.c, and this new test would have been introduced in r7383 for ticket #2510: 

  psSHP->nRecords = pabyBuf[27] + pabyBuf[26] * 256 + pabyBuf[25] * 256 * 256 + pabyBuf[24] * 256 * 256 * 256;
  psSHP->nRecords = (psSHP->nRecords*2 - 100) / 8;
  
  if( psSHP->nRecords < 0 || psSHP->nRecords > 256000000 )
  {
    msSetError(MS_SHPERR, "Corrupted .shp file : nRecords = %d.", "msSHPOpen()",
               psSHP->nRecords);
...

Since the file is empty, the bytes read in the header in the first line above result in nRecords=0, and then the value is adjusted on the following line to nRecords = (0 * 2 -100)/8 = -12 ... and then we get the error that was reported by nsavard.

Steve, Even, what do you suggest?

comment:3 by dmorissette, 16 years ago

The following change fixes the error, but is this going to be safe for any shapefile? 

--- mapshape.c  (revision 7779)
+++ mapshape.c  (working copy)
@@ -292,7 +292,8 @@
   }
   
   psSHP->nRecords = pabyBuf[27] + pabyBuf[26] * 256 + pabyBuf[25] * 256 * 256 + pabyBuf[24] * 256 * 256 * 256;
-  psSHP->nRecords = (psSHP->nRecords*2 - 100) / 8;
+  if (psSHP->nRecords != 0)
+      psSHP->nRecords = (psSHP->nRecords*2 - 100) / 8;
   
   if( psSHP->nRecords < 0 || psSHP->nRecords > 256000000 )
   {

comment:4 by dmorissette, 16 years ago

Resolution: → fixed
Status: assigned → closed

No comments, so I've committed the test above in r7789 to avoid the fatal error when the file is empty.

Note: See TracTickets for help on using tickets.