Ticket #1374 (closed defect: fixed)

Opened 7 years ago

Last modified 7 years ago

ESRI Style WKT (shape.prj file) not read

Reported by: neteler Owned by: warmerdam
Priority: highest Milestone:
Component: OGR_SF Version: unspecified
Severity: blocker Keywords:
Cc:

Description

Frank,

the attached SHAPE file in Italian Monte Mario Gauss Boaga Zone 1
projection comes with a .prj file which is (no longer?) read by
OGR:

ogrinfo -so comuniBL_from_geocomuni.shp comuniBL_from_geocomuni
INFO: Open of `comuniBL_from_geocomuni.shp'
      using driver `ESRI Shapefile' successful.

Layer name: comuniBL_from_geocomuni
Geometry: Polygon
Feature Count: 69
Extent: (1706331.999377, 5084611.000299) - (1787506.251707, 5175636.000384)
Layer SRS WKT:
(unknown)
...

The file reads as:
cat comuniBL_from_geocomuni.prj
PROJCS["Transverse Mercator",GEOGCS["international",DATUM["D_Monte_Mario",SPHEROID["International_1924",6378388,297]],PRIMEM["Greenwich",0],UNIT["Degree",0.017453292519943295]],PROJECTION["Transverse_Mercator"],PARAMETER["latitude_of_origin",0],PARAMETER["central_meridian",9],PARAMETER["scale_factor",0.9996],PARAMETER["false_easting",1500000],PARAMETER["false_northing",0],UNIT["Meter",1]]

which looks reasonable to me.

Best regards,
Markus

Attachments

comuni.tar.gz Download (246.4 KB) - added by neteler@… 7 years ago.
SHAPE with failing .prj file

Change History

Changed 7 years ago by neteler@…

SHAPE with failing .prj file

Changed 7 years ago by neteler@…

Frank,

it appears that a CVS update and recompile of OGR cured the problem.
Closing the report.

Markus

Changed 7 years ago by warmerdam

My favorite - a bug that closes itself!

Changed 7 years ago by neteler@…

Frank,

bad news: back home the bug is back, even after fresh recompile.
The difference is that I am using a gcc4 compiler here:

cat rivers_vmap0_BL.prj
PROJCS["Transverse Mercator",GEOGCS["international",DATUM["D_Monte_Mario",SPHEROID["International_1924",6378388,297]],PRIMEM["Greenwich",0],UNIT["Degree",0.017453292519943295]],PROJECTION["Transverse_Mercator"],PARAMETER["latitude_of_origin",0],PARAMETER["central_meridian",9],PARAMETER["scale_factor",0.9996],PARAMETER["false_easting",1500000],PARAMETER["false_northing",0],UNIT["Meter",1]][neteler@localhost belluno]$
[neteler@localhost belluno]$ ogrinfo -so rivers_vmap0_BL.shp rivers_vmap0_BL
INFO: Open of `rivers_vmap0_BL.shp'
      using driver `ESRI Shapefile' successful.

Layer name: rivers_vmap0_BL
Geometry: Line String
Feature Count: 37
Extent: (1711726.803917, 5086738.184453) - (1785579.845740, 5171219.664505)
Layer SRS WKT:
(unknown)
cat: Real (11.0)
cat_: Real (24.15)
id: Real (24.15)
f_code: String (80.0)
hyc: Real (24.15)
nam: String (80.0)
tile_id: Real (24.15)
edg_id: Real (24.15)

gcc -v
Using built-in specs.
Target: i586-mandriva-linux-gnu
Configured with: ../configure --prefix=/usr --libexecdir=/usr/lib --with-slibdir=/lib --mandir=/usr/share/man --infodir=/usr/share/info --enable-shared --enable-threads=posix --enable-checking=release --enable-languages=c,c++,ada,fortran,objc,obj-c++,java --host=i586-mandriva-linux-gnu --with-cpu=generic --with-system-zlib --enable-long-long --enable-__cxa_atexit --enable-clocale=gnu --disable-libunwind-exceptions --enable-java-awt=gtk --with-java-home=/usr/lib/jvm/java-1.4.2-gcj-1.4.2.0/jre --enable-gtk-cairo --enable-ssp --disable-libssp
Thread model: posix
gcc version 4.1.1 20060724 (prerelease) (4.1.1-3mdk)

I don't know how to debug that. If needed, I can grant you access to
my laptop (via ADSL).

sorry,
Markus

Changed 7 years ago by warmerdam

Swapnil,

Could you work with Markus to try and reproduce this and fix it? 

Shapefile .prj file reading is accomplished circa line 364 of
gdal/ogr/ogrsf_frmts/shape/ogrshapedatasource.cpp.  But I think
the first challenge will be to reproduce the problem. 

Changed 7 years ago by neteler@…

Frank, Swapnil,

this problem as well as the LOCALE problem appeared only after
system upgrade to Mandriva2007 which ships with
gcc version 4.1.1 20060724 

Do you have a setup with gcc4.1 to test it? As offered,
I can give you ssh access to my laptop (Frank did that already
2-3 years ago to fix OGDI).

Markus

Changed 7 years ago by dreamil@…

Markus,
  I could reproduce the problem on my Debian testing/unstable. I have gcc 4.0.3
  So I guess my machine would be sifficient to debug the problem.

Changed 7 years ago by dreamil@…

Hi,
  I could find out that the problem is that ogrinfo is not recognizing the prj file because of UNIT["Meter",1] at the end of prj file. I removed this part and it worked in ogrinfo showing all SRS info. Also to be noted is that the original shape file with Markus' prj file does not show any SRS info in QGIS also. When I tried opening the shape file with modified prj file in QGIS, it showes the SRS correctly. Is this UNIT["Meter",1] a new addition to prj file or is altogether wrong? 
  Because of this problem, the importFromWkt() function in ogr_srsnode.cpp returns OGRERR_CORRUPT_DATA.

Changed 7 years ago by neteler@…

Hi,

I just exported another map from my GRASS database to SHAPE file
and it contains this UNIT["Meter",1] piece.

The GRASS location settings are:
# standard style:
GRASS 6.3.cvs (belluno):~/disstext/zecche/belluno_ticks_model > g.proj -w
PROJCS["Transverse Mercator",
    GEOGCS["international",
        DATUM["Monte_Mario",
            SPHEROID["International_1924",6378388,297],
            TOWGS84[-104.1,-49.1,-9.9,0.971,-2.917,0.714,-11.68]],
        PRIMEM["Greenwich",0],
        UNIT["degree",0.0174532925199433]],
    PROJECTION["Transverse_Mercator"],
    PARAMETER["latitude_of_origin",0],
    PARAMETER["central_meridian",9],
    PARAMETER["scale_factor",0.9996],
    PARAMETER["false_easting",1500000],
    PARAMETER["false_northing",0],
    UNIT["meter",1]]

# proj4 style:
GRASS 6.3.cvs (belluno):~/disstext/zecche/belluno_ticks_model > g.proj -p
-PROJ_INFO-------------------------------------------------
name       : Transverse Mercator
datum      : rome40
datumparams: towgs84=-104.1,-49.1,-9.9,0.971,-2.917,0.714,-11.68
proj       : tmerc
ellps      : international
a          : 6378388.0000000000
es         : 0.0067226700
f          : 297.0000000000
lat_0      : 0.0000000000
lon_0      : 9.0
k_0        : 0.9996000000
x_0        : 1500000.0000000000
-PROJ_UNITS------------------------------------------------
unit       : meter
units      : meters
meters     : 1.0

which is the standard.

# ESRI style
GRASS 6.3.cvs (belluno):~/disstext/zecche/belluno_ticks_model > g.proj -we
PROJCS["Transverse Mercator",
    GEOGCS["international",
        DATUM["D_Monte_Mario",
            SPHEROID["International_1924",6378388,297]],
        PRIMEM["Greenwich",0],
        UNIT["Degree",0.017453292519943295]],
    PROJECTION["Transverse_Mercator"],
    PARAMETER["latitude_of_origin",0],
    PARAMETER["central_meridian",9],
    PARAMETER["scale_factor",0.9996],
    PARAMETER["false_easting",1500000],
    PARAMETER["false_northing",0],
    UNIT["Meter",1]]

The PROJ/OGR code in GRASS relies on these external libraries.
The v.out.ogr wasn't changed for a long time.

Markus

Changed 7 years ago by warmerdam

The UNIT["Meter",1] construction is common, I don't see any inherent problem 
with it.  Swapnil will need to debug through in detail to see what is going
wrong.

Changed 7 years ago by neteler@…

Hi,

I can also confirm this bug on
gcc version 3.4.6 20060404 (Red Hat 3.4.6-3)
Red Hat Enterprise Linux WS release 4 (Nahant Update 4)

Markus

Changed 7 years ago by dreamil@…

Hi 
  I kind of rectified the problem. What I did was I changed the gdal/port/cpl_conv.cpp line 765 from 
             while( nChunkBytesConsumed < nChunkBytesRead-1 && !bBreak )
To
             while( nChunkBytesConsumed <= nChunkBytesRead-1 && !bBreak )

before this, the loop was skipping remaining ']' character at the end of prj file which was causing corrupt data error.

I tested the ogrinfo for both Markus' prj file (which was not accepted earlier) as well as my own prj file (which was accepted earlier). 

But I could not make out much from the CPLReadLineL code :-(
Anyway, frank it is up to you to decide whether this solution is accepted or not. I have not yet committed the code to CVS.

Bye

Changed 7 years ago by neteler@…

Thanks, it now works.

Markus

Changed 7 years ago by neteler@…

Fixed :-)

Changed 7 years ago by mloskot

Unfortunately, I have to reopen this bug because the reading .prj file seems to be still broken.

After the fix has been applied to cpl_conv.cpp:772:

while( nChunkBytesConsumed <= nChunkBytesRead-1 && !bBreak )

every reading of .prj file, at some point, hooks return NULL; instruction
in the line 764, because 0 bytes are read from a .prj file:

nChunkBytesRead = VSIFReadL( szChunk, 1, nChunkSize, fp );
if( nChunkBytesRead == 0 )
    return NULL;

Replacing '<=' with '<' in line 772 seems to fix the problem, but not exactly because in this case files attached by Markus give the same bug again.

I found that if I add LF character (newline) at the and of /comuniBL_from_geocomuni.prj file, the problem disappears and files from Markus work well (with old '<' condition in line 772).

Here is diff output for original file from Markus and my version with added newline character:

mloskot:~/tmp$ diff comuniBL_from_geocomuni.prj comuniBL_from_geocomuni_test.prj
1c1
< PROJCS["Transverse Mercator",GEOGCS["international",DATUM["D_Monte_Mario",SPHEROID["International_1924",6378388,297]],PRIMEM["Greenwich",0],UNIT["Degree",0.017453292519943295]],PROJECTION["Transverse_Mercator"],PARAMETER["latitude_of_origin",0],PARAMETER["central_meridian",9],PARAMETER["scale_factor",0.9996],PARAMETER["false_easting",1500000],PARAMETER["false_northing",0],UNIT["Meter",1]]
\ No newline at end of file
---
> PROJCS["Transverse Mercator",GEOGCS["international",DATUM["D_Monte_Mario",SPHEROID["International_1924",6378388,297]],PRIMEM["Greenwich",0],UNIT["Degree",0.017453292519943295]],PROJECTION["Transverse_Mercator"],PARAMETER["latitude_of_origin",0],PARAMETER["central_meridian",9],PARAMETER["scale_factor",0.9996],PARAMETER["false_easting",1500000],PARAMETER["false_northing",0],UNIT["Meter",1]]

As I explained, the comuniBL_from_geocomuni_test.prj file works without any problems.

After some analysis and chat with Frank, it seems the conclusion is that the algorithm for reading .prj files in CPLReadLineL is "unstable".

Frank asked me to reopen this bug and assign it back to him.

Changed 7 years ago by warmerdam

It turns out the aaigrid.py related problem was not related to Swapnil's 
patch, but was due to a post GDAL 1.4.0RC1 patch I made to CPLReadLineL() 
to return NULL at EOF.  It was also occuring prematuring for files with no
newlines at all. 

I have corrected this problem in CVS, and luckily it did not affect RC1. 

Unfortunately, on inspection I see there is a "read one byte past end of
buffer" problem with Swapnil's last fix.  This will only very very rarely
trigger any sort of problem.  It will occur if a newline happens to 
occur just at the end of the chunk buffer, and the indeterminate memory
following the chunk buffer happens to have a newline character too, then 
a two character skip may occur. 

Because this is such a rare circumstance I don't think the 1.4.0 
release needs to be held for it. 

I am leaving this bug open though as Swapnil's patch and the original
bug need to be reconsidered.

Changed 7 years ago by warmerdam

I have corrected the bug and tested with the following test harnass. 

#include "cpl_conv.h"

#define MAX_LEN 500

/************************************************************************/
/*                                Test()                                */
/************************************************************************/

static void Test( const char *pszExpected, const char *pszWriteBuf )

{
    FILE *fp = fopen( "test.txt", "w" );

    fwrite( pszWriteBuf, 1, strlen(pszWriteBuf), fp );
    fclose( fp );

    fp = VSIFOpenL( "test.txt", "r" );
    const char *pszLine = CPLReadLineL( fp );
    VSIFCloseL( fp );

    if( strcmp(pszLine,pszExpected) != 0 )
    {
        printf( "Error!\n  Expected:'%s'\n  Got:     '%s'\n", 
                pszExpected, pszLine );
    }
}

/************************************************************************/
/*                                main()                                */
/************************************************************************/

int main()

{
    int iLen;
    char szExpected[MAX_LEN+2];
    char szWriteBuf[MAX_LEN+20];

    for( iLen = 0; iLen < sizeof(szExpected)-3; iLen++ )
    {
        szExpected[iLen] = 'a' + (iLen % 26);
        szExpected[iLen+1] = '\0';

        strcpy( szWriteBuf, szExpected );
        Test( szExpected, szWriteBuf );
        
        strcat( szWriteBuf, "\n" );
        Test( szExpected, szWriteBuf );
        
        strcat( szWriteBuf, "abc\n" );
        Test( szExpected, szWriteBuf );

        strcpy( szWriteBuf, szExpected );
        strcat( szWriteBuf, "\r\n" );
        Test( szExpected, szWriteBuf );
        
        strcat( szWriteBuf, "abc\r\n" );
        Test( szExpected, szWriteBuf );

        strcpy( szWriteBuf, szExpected );
        strcat( szWriteBuf, "\r" );
        Test( szExpected, szWriteBuf );
        
        strcat( szWriteBuf, "abc\r" );
        Test( szExpected, szWriteBuf );

        strcpy( szWriteBuf, szExpected );
        strcat( szWriteBuf, "\r" );
        Test( szExpected, szWriteBuf );
        
        strcat( szWriteBuf, "abc\r" );
        Test( szExpected, szWriteBuf );

        fprintf( stdout, "." );
        fflush( stdout );
    }

    fprintf( stdout, "\n" );
}


Note: See TracTickets for help on using tickets.