Parsing error when loading maps with quoted connection strings

[szekerest]

Some data sources require to quote identifiers with special characters like 
table names. Connections strings containing double quotes produce parsing 
error. However setting this type of connection string at run time seems to be 
working correctly.

Tamas Szekeres

[sdlime]

Can you post some examples?

Steve

[szekerest]

Here is the layer definition that could not be parsed. The connection 
parameter was set at run time usig mapscript, and the map file was saved using 
mapObj.save()

LAYER
    CONNECTION "host=localhost port=5433 dbname=GISData user='postgres'"
    CONNECTIONTYPE POSTGIS
    DATA "wkb_geometry from "Gráf/lines wkt" using unique ogc_fid using 
SRID=4326"
      METADATA
      END
    NAME "Gráf/lines wkt"
    PROJECTION
      "init=EPSG:4326"
    END
    SIZEUNITS PIXELS
    STATUS ON
    TOLERANCEUNITS PIXELS
    TYPE LINE
    UNITS METERS
    CLASS
      NAME "Gráf/lines wkt"
      LABEL
        SIZE MEDIUM
        TYPE BITMAP
        BUFFER 0
        COLOR 0 0 0
        FORCE TRUE
        MINDISTANCE -1
        MINFEATURESIZE -1
        OFFSET 0 0
        PARTIALS TRUE
        POSITION CC
      END
      METADATA
      END
      STYLE
        ANGLE 360
          COLOR 42 254 188
          OUTLINECOLOR 69 75 171
        SYMBOL 0
      END
      TEMPLATE "query.html"
    END
  END


Tamas Szekeres

[sdlime]

The problem isn't the parser then, it should break on the example you sent. The 
problem is the map file writer. It should be smarter in choosing the quote type 
to use when writing that element. Can you use single qoutes in the DATA element 
when it is originally defined as a workaround?

Steve 

[szekerest]

PostgreSQL supports only double quotes with idetifiers. This applies to MSSQL 
as well.

Tamas

[szekerest]

I think the parser should utilize escape characters with strings, or read the 
string until the next newline.

Tamas

[sdlime]

Escaping quotes is a good idea and has been on my todo list for ages. It never 
seems to be a big enough deal to work on. Just making the writer smarter would 
help you in this case. 

Reading to the next newline isn't a good idea in my opinion.

Steve

[dmorissette]

Setting target milestone to 4.10 as this seems important to fix for this release.

Tamas, I think what Steve meant was that in your mapfile def'n you could have
used single quotes as teh mapfile string delimiter:

  DATA 'wkb_geometry from "Gráf/lines wkt" using unique ogc_fid using SRID=4326'

and a quick fix for this specific case would be to modify writeLayer()
(mapfile.c) to write the DATA statement this way (untested code):

  if(layer->data) { 
    if (strchr(layer->data, '\"') != NULL)
      fprintf(stream, "    DATA '%s'\n", layer->data);
    else
      fprintf(stream, "    DATA \"%s\"\n", layer->data);
  } 

The same should probably be done for the CONNECTION parameter.

Of course, introducing a syntax to properly escape quotes would be best, the
above is just a quick fix until escaping is in place.

[sdlime]

Tamas: Is this still an issue? If so we can try to squeeze in for 5.0.

Steve

[dmorissette]

Does the escaping done by Umberto in ticket #2123 address this as well?

[unicoletti]

Still does not work using the example posted by Tamas. Mapscript bails out with the following error:

m = mapscript.mapObj('test2.map')
Traceback (most recent call last):

File "<stdin>", line 1, in <module>
File "/usr/lib/python2.5/site-packages/mapscript.py", line 757, in init

this = _mapscript.new_mapObj(*args)

_mapscript.MapServerError: loadLayer(): Unknown identifier. Parsing error near (Graf/lines):(line 244)

I have tried with both lines (line 244), same error:

DATA "wkb_geometry from \"Graf/lines wkt\" using unique ogc_fid using SRID=4326" #DATA "wkb_geometry from "Graf/lines wkt" using unique ogc_fid using SRID=4326"

It should not be too difficult to fix this though.

[unicoletti]

I took a look and the problem is in the MS_STRING part of the lexer (line 468-473) which does not seem to accept double quotes (") inside the string itself, but it is quite happy with single quotes (') instead. The following DATA statement was successfully parsed (dont't know if postgres will accept it as I don't have the data):

DATA "wkb_geometry from 'Graf/lines wkt' using unique ogc_fid using SRID=4326"

To use the quotes escaping tricks from #2123 it should be enough to implement the comment at line 501 of maplexer.l and then modify the string parsing code like I have done for EXPRESSION_STRING at 483 of the same file.

[unicoletti]

I'm in verbose mode today...

This page could be of help if somebody had the time (not me, unfortunately) to work on this, in particular see the notes on the state stack at the end:

​http://www.gnu.org/software/flex/manual/html_chapter/flex_11.html

[unicoletti]

Since the connection string is parsed using the following rule by the parser:

<INITIAL,URL_STRING>\"[^\"]*\"|\'[^\']*\'

it is, well, clear that double quotes are allowed inside single quotes and viceversa, so Tamas could try to rewrite the connection string as the following:

DATA 'wkb_geometry from "Graf/lines wkt" using unique ogc_fid using SRID=4326'

and confirm if this is a valid workaround.

[sdlime]

Tamas/Umberto: Any more work needed on this or can I close? Otherwise please mark as candidate for 5.2. Thanks.

Steve

[unicoletti]

Waiting for feedback from Tamas...

[tamas]

It seems that the single quoting is working pretty well with the parser:

DATA 'wkb_geometry from "Graf/lines wkt" using unique ogc_fid using SRID=4326'

But the mapfile writer replaces the single quotes with double quotes as:

DATA "wkb_geometry from "Graf/lines wkt" using unique ogc_fid using SRID=4326"

so loading the mapfile results in a parsing error.

I think it would be sufficient to apply such fix that Daniel have been mentioned before.

[tamas]

I've tested Daniel's fix and seems ok.

[sdlime]

I applied the temporary fix to DATA and CONNECTION writing. Ultimately I think we could have a generic encode/decode capability to deal with quotes. Heck, even a generic writeString() function would work ok. That way all strings could be checked for appropriate Can open a new bug on that and call this one closed. r6784

I also updated writeExpression to do the same thing with string expressions... (r6785)

Steve