Ticket #2674 (closed defect: fixed)

Opened 2 months ago

Last modified 2 months ago

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

Reported by: nsavard Assigned to: 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

07/03/08 14:18:05 changed by dmorissette

  • status changed from new to assigned.
  • owner changed from mapserverbugs to dmorissette.
  • description changed.
  • cc set to assefa, sdlime.

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

07/03/08 14:33:11 changed by dmorissette

  • cc changed from assefa, sdlime to assefa, sdlime, rouault, warmerdam.

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?

07/03/08 14:38:23 changed by dmorissette

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 )
   {

07/07/08 16:07:45 changed by dmorissette

  • status changed from assigned to closed.
  • resolution set to fixed.

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