setFilter expression is truncated after 255 characters

[tawood@…]

The setFilter method of the ms_newLayerObj truncates the expression argument at
the 255th (254th?) character. This creates a "bad" query and breaks the layer.

Thanks!

[dmorissette]

I didn't try to reproduce this yet, but I had a quick look at the code in
loadExpressionString() (mapfile.c) and didn't see any fixed limit on the lenght
of the expression strings that would explain that.

Adding Steve to CC: Steve, are you aware of any 255 chars limit on expression
strings in the parser?

[sdlime]

It's possible there is a token maximum for Flex. I've got a book on that beast 
at home and can check later today. I would imagine it could be overriden...

Steve

[sdlime]

Flex has a default token length of 8K so that's not the problem, not unless some
other derivitive of lex is being used to build. If you roll the same mapfile by
hand with the long FILTER does it still break?

Steve

[sdlime]

Well, I can verify that long filters or expressions defined within the mapfile
work ok. I played with one 330 characters long with no problem. Pass that same
string via setFilter and it gets truncated. I think the problem is in maplexer.l
in the regular expressions someplace. *ugh*

Steve

[sdlime]

Latest flex still exhibits the problem. What's happening is that somewhere
within the lexer the buffer or it's size is getting changed. It's not in the
preparation of the string within flex, it get's to the scanner intact- I've
traced it that far. I can't find the place where the truncation is happening
though, flex output is not really readable, go figure. Turning on scanner
debuging confirms the truncation happens and the wrong rule is matched.

Dan, any ideas? This one could be a real time sink...

Terry, the workaround at the moment would be to write a mapfile to disk with the
 filter embedded and then load it (e.g. avoid setFilter). In my tests the long
filter is parsed correctly from a file.

I changed the component to the C library since this is not PHP specific.

Steve

[tawood@…]

Steve,
  Has anything more been discovered about this problem?

Thanks,
  Terry Wood

[dmorissette]

In response to Steve's question in comment #5, no, I do not really have anything
to suggest unfortunately, stepping through the code like you did is the best I
could have done too.

(Reassigned bug to Steve)

[sdlime]

Nope. I could reproduce the bug but could never find the cause. It needs a 
second set of eyes. Let me throw it out there to the mapserver-dev list and see 
if anyone wants a crack at it. Thanks for the reminder.

Steve

[sdlime]

Nope. I could reproduce the bug but could never find the cause. It needs a 
second set of eyes. Let me throw it out there to the mapserver-dev list and see 
if anyone wants a crack at it. Thanks for the reminder.

Steve

[b.veldkamp@…]

Using PHP MapScript, setting the expression works fine,
$class->setExpression($verylongstring), but I can't *get* the expression
$class->getExpression(). It seems to crash when expression is over 512 characters.

Here's what my code looks like:

// All expressions are initially set to "empty"
while(odbc_fetch_row($records))
{  $rikzcode = odbc_result ($records, odbc_field_name ($records, 1));
  $value = odbc_result ($records, odbc_field_name ($records, 2));

  // Determine class to which this ID belongs
  $classID = GetClassIDByValue($value);

  if ($classID > -1)
  {
    $cls = $layer->getClass($classID);
    $exp = $cls->getExpression();  // <== Crash here

    if ($exp == "\"empty\"")
      $exp = "( \"[RIKZCODE]\" IN
\"$rikzcode\" )"; // initial value
    else
      $exp = substr($exp, 0, -3) . ",$rikzcode\" )"; // append value

    $cls->setExpression($exp);
  }
}

[sdlime]

Ok, I think I found it. Whomever wrote the getExpressionString method in
MapScript uses a statically allocated variable to hold the expression string
built by getExpressionString rather than looking at the size of the expression
internals. PHP uses a 512K buffer and SWIG a 256K buffer - the sizes reported as
failing.

I didn't find this with the original bug because there was no mention of using
getExpressionString although that must be part of what Terry saw. My test script
used getExpressionString to look at the set expression. I always figured it was
screwed up going in, but it's the going out part. You can set a big filter or
expression and use the save method to write the mapfile and you'll see the long
expression fine in the output.

Will fix shortly (off to bed now). Dan can you fix PHP based on my fix? (BTW we
should add cases for case insensitive queries too) 

Steve

[dmorissette]

Fixing this by moving the code down to a msGetExpressionString() function in
mapfile.c. Will commit to CVS shortly and you'll just need to update your
class.i and retest.

[dmorissette]

okay, let me reword my last comment.

I have implemented a new msGetExpressionString() in mapfile.c with the code that
was duplicated in the mapscript wrappers and fix that Steve suggested (malloc
based on expression length + handle the MS_EXP_INSENSITIVE case).

I have tested the fix with PHP MapScript (and with Valgrind), all looks good.

I have updated swiginc/class.i to call msGetExpressionString() but could not
test. I'll leave that to you Steve and then you can close the bug.

BTW, while looking at class.i, I noticed a getTextString() method that suffers
of the same problem. PHP is fine because it simply doesn't provide that method. ;)

[sdlime]

What's funny is I wrote the exact same fix last night. Ran into lots of
conflicts then when updating. ;-) I stuck with your version of
msGetExpressionString(). I took it a bit further. Sean had written a
msLayerGetFilterString function. It's not needed anymore and wasn't complete
anyway. So: 

  - I removed that function
  - switched layer.i and mapscript_i.c to use msGetExpressionString for filters too

I noticed one inconsistency between PHP and Swig MapScript. You use getFilter
while Swig uses getFilterString (both use getExpressionString in a class).

I can fix getTextString too...

Steve

[dmorissette]

Oopps... sorry, I didn't mean to duplicate your work. When I read "Will fix
shortly (off to bed now). Dan can you fix PHP based on my fix?" I went to look
at the code and since I didn't find your fix I figured that you had not done it
yet and I decided to implement the fix since I was already there. I should have
waited I guess.

I have created bug 1892 to fix the inconsistency in the method names in 5.0

[sdlime]

Marking as fixed now...

Steve