Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Label using TEXT shows field name instead of field contents #4012

Closed
mapserver-bot opened this issue Apr 4, 2012 · 15 comments
Closed

Label using TEXT shows field name instead of field contents #4012

mapserver-bot opened this issue Apr 4, 2012 · 15 comments
Assignees
Milestone

Comments

@mapserver-bot
Copy link

Reporter: saharlan
Date: 2011/09/08 - 15:35
Trac URL: http://trac.osgeo.org/mapserver/ticket/4012
I am labeling a simple line using the TEXT directive in a CLASS element. The field used to label is called NAME and I am adding a text prefix to the label. The line "TEXT 'Bus [NAME]'" labels the line correctly. However, if I change the word 'Bus' to the word 'Loop', the map label does not display the contents of the field NAME. Instead, it displays the name of the field.

@mapserver-bot
Copy link
Author

Author: saharlan
Date: 2011/09/08 - 15:38
Command used to produce bug:
shp2img.exe -m test.map -o test.png -s 400 400 -e -92.69 37.64 -92.63 37.67

@mapserver-bot
Copy link
Author

attachment http://trac.osgeo.org/mapserver/attachment/ticket/4012/msbugrep.zip :

   Zipped files to reproduce bug. Use SHP2IMG with example line in 'map' file.

@ghost ghost assigned sdlime Apr 5, 2012
@tbonfort
Copy link
Member

tbonfort commented Jul 2, 2012

Wow, this one is weird, I was able to reproduce with master. @sdlime this seems parser related, can you have a look? (The supplied testcase works out of the box)

@tbonfort
Copy link
Member

tbonfort commented Jul 2, 2012

these are some tests I've run, with the string passed to TEXT first, and the resulting output second:

Louri[NAME] => Louri44
Loupi[NAME] => Loupi[NAME]
Loup[NAME] => Loup[NAME]
valgrind shows an unfreed malloc which doesn't happen with "Lour[NAME]":

==17966== 80 bytes in 1 blocks are definitely lost in loss record 8 of 50
==17966==    at 0x4C28BED: malloc (vg_replace_malloc.c:263)
==17966==    by 0x4F782B5: msTokenizeExpression (maplayer.c:362)
==17966==    by 0x4F799A2: msLayerWhichItems (maplayer.c:591)
==17966==    by 0x4F9BE2F: msDrawVectorLayer (mapdraw.c:937)
==17966==    by 0x4F9B79E: msDrawLayer (mapdraw.c:808)
==17966==    by 0x4F9A5F6: msDrawMap (mapdraw.c:437)
==17966==    by 0x4023F6: main (shp2img.c:293)

Louq[NAME] => segfault

(gdb) bt
#0  *__GI___libc_free (mem=0xc11) at malloc.c:3709
#1  0x00007ffff7ab2580 in msFreeShape (shape=0x7ffff3455eb8) at mapprimitive.c:160
#2  0x00007ffff7a87b06 in freeExpressionTokens (exp=0x6109e8) at mapfile.c:2184
#3  0x00007ffff7ad0e94 in msLayerClose (layer=0x60f760) at maplayer.c:223
#4  0x00007ffff7af5e58 in msDrawVectorLayer (map=0x607e00, layer=0x60f760, image=0x6371f0)
    at mapdraw.c:1183
#5  0x00007ffff7af479f in msDrawLayer (map=0x607e00, layer=0x60f760, image=0x6371f0)
    at mapdraw.c:808
#6  0x00007ffff7af35f7 in msDrawMap (map=0x607e00, querymap=0) at mapdraw.c:437
#7  0x00000000004023f7 in main (argc=13, argv=0x7fffffffe758) at shp2img.c:293

when run inside valgrind the code doesn't segfault, but there's an additional message:

==18145== Conditional jump or move depends on uninitialised value(s)
==18145==    at 0x4F59550: msFreeShape (mapprimitive.c:157)
==18145==    by 0x4F2EB05: freeExpressionTokens (mapfile.c:2184)
==18145==    by 0x4F77E93: msLayerClose (maplayer.c:223)
==18145==    by 0x4F9CE57: msDrawVectorLayer (mapdraw.c:1183)
==18145==    by 0x4F9B79E: msDrawLayer (mapdraw.c:808)
==18145==    by 0x4F9A5F6: msDrawMap (mapdraw.c:437)
==18145==    by 0x4023F6: main (shp2img.c:293)
==18145== 
==18145== Conditional jump or move depends on uninitialised value(s)
==18145==    at 0x4C27D04: free (vg_replace_malloc.c:427)
==18145==    by 0x4F2EB15: freeExpressionTokens (mapfile.c:2185)
==18145==    by 0x4F77E93: msLayerClose (maplayer.c:223)
==18145==    by 0x4F9CE57: msDrawVectorLayer (mapdraw.c:1183)
==18145==    by 0x4F9B79E: msDrawLayer (mapdraw.c:808)
==18145==    by 0x4F9A5F6: msDrawMap (mapdraw.c:437)
==18145==    by 0x4023F6: main (shp2img.c:293)

Lour[NAME] => Lour44

I have also tried all the other letters of the alphabet, i.e. Lou[a-z][NAME]: only Loup and Louq cause issues.

Lou[NAME] => Lou44
[NAME]Loup => 44Loup

@sdlime
Copy link
Member

sdlime commented Jul 3, 2012

Will have a look... May be a token processing issue.

@tbonfort
Copy link
Member

tbonfort commented Jul 3, 2012

debug output when using Loup[NAME]:

$ shp2img -m test.map -o test.png -s 400 400 -e -92.69 37.64 -92.63 37.67
msDrawMap(): rendering using outputformat named png (AGG/PNG).
msParseTime(): Regular expression error. Unrecognized date or time format ().
msTokenizeExpression(): Expression parser error. Parsing time value failed.
msDrawMap(): Layer 0 ((null)), 0.001s
msDrawMap(): Drawing Label Cache, 0.000s
msDrawMap() total time: 0.002s
msSaveImage(test.png) total time: 0.009s
msFreeMap(): freeing map at 0x14f2e00.
freeLayer(): freeing layer at 0x14fa8a0.

@tbonfort
Copy link
Member

tbonfort commented Jul 3, 2012

some further debugging: in msTokenizeExpression() the code goes through case MS_TOKEN_LITERAL_TIME: which is enumed at 112. 112 is also the ascii char for 'p'

@tbonfort
Copy link
Member

tbonfort commented Jul 3, 2012

ccing @aboudreault as is seems that this could be related to 95cb999. The following patch fixes the issue for me, but it surely has some side-effects I am not aware of.


diff --git a/maplexer.l b/maplexer.l
index dd1d74e..8fd3f33 100644
--- a/maplexer.l
+++ b/maplexer.l
@@ -698,7 +698,12 @@ char path[MS_MAXPATHLEN];
                                                   strcpy(msyystring_buffer, msyytext); 
                                                   return(0); 
                                                 }
-<EXPRESSION_STRING>.                            { return(msyytext[0]); }
+<EXPRESSION_STRING>.                            { 
+                                                  MS_LEXER_STRING_REALLOC(msyystring_buffer, strlen(msyytext), 
+                                                                          msyystring_buffer_size, msyystring_buffer_ptr);
+                                                  strcpy(msyystring_buffer, msyytext); 
+                                                  return(MS_STRING); 
+                                                }
 %%

 /*

without this patch, msyylex() returns a single character instead of a token, which completely messes up the logic in msTokenizeExpression() . I'm surprised this hasn't been brought up earlier as this isn't an unusual use-case.

@aboudreault
Copy link
Member

well... the original code seems to do what it should... returning a single char. The question is why the string was not caught by the previous regex...

@sdlime
Copy link
Member

sdlime commented Aug 7, 2012

Who owns this one now? Steve

@tbonfort
Copy link
Member

tbonfort commented Aug 7, 2012

@sdlime it's all your's if you can have a go at it

@tbonfort
Copy link
Member

tbonfort commented Aug 7, 2012

some more tests that might help debugging: it seems that whenever class->text is of the form {string1}[attribute]{string2} then whenever {string1} or {string2} contains a 'p' (MS_TOKEN_LITERAL_TIME) or 'q' (MS_TOKEN_LITERAL_SHAPE) then the error will occur. Note that string1 or string2 can be empty.

@tbonfort
Copy link
Member

tbonfort commented Aug 7, 2012

The following diff solves this issue, by ensuring that a single character returned by msyylex does not collide with one of our enum'd tokens.

diff --git a/mapserver.h b/mapserver.h
index fed2467..8c20ff9 100644
--- a/mapserver.h
+++ b/mapserver.h
@@ -635,20 +635,20 @@ extern "C" {
   /*                     expressionObj & tokenObj                         */
   /************************************************************************/

-  enum MS_TOKEN_LOGICAL_ENUM { MS_TOKEN_LOGICAL_AND=100, MS_TOKEN_LOGICAL_OR, MS_TOKEN_LOGICAL_NOT };
-  enum MS_TOKEN_LITERAL_ENUM { MS_TOKEN_LITERAL_NUMBER=110, MS_TOKEN_LITERAL_STRING, MS_TOKEN_LITERAL_TIME, MS_TOKEN_LITERAL_SHAPE };
+  enum MS_TOKEN_LOGICAL_ENUM { MS_TOKEN_LOGICAL_AND=300, MS_TOKEN_LOGICAL_OR, MS_TOKEN_LOGICAL_NOT };
+  enum MS_TOKEN_LITERAL_ENUM { MS_TOKEN_LITERAL_NUMBER=310, MS_TOKEN_LITERAL_STRING, MS_TOKEN_LITERAL_TIME, MS_TOKEN_LITERAL_SHAPE };
   enum MS_TOKEN_COMPARISON_ENUM {
-    MS_TOKEN_COMPARISON_EQ=120, MS_TOKEN_COMPARISON_NE, MS_TOKEN_COMPARISON_GT, MS_TOKEN_COMPARISON_LT, MS_TOKEN_COMPARISON_LE, MS_TOKEN_COMPARISON_GE, MS_TOKEN_COMPARISON_IEQ,
+    MS_TOKEN_COMPARISON_EQ=320, MS_TOKEN_COMPARISON_NE, MS_TOKEN_COMPARISON_GT, MS_TOKEN_COMPARISON_LT, MS_TOKEN_COMPARISON_LE, MS_TOKEN_COMPARISON_GE, MS_TOKEN_COMPARISON_IEQ,
     MS_TOKEN_COMPARISON_RE, MS_TOKEN_COMPARISON_IRE,
     MS_TOKEN_COMPARISON_IN, MS_TOKEN_COMPARISON_LIKE,
     MS_TOKEN_COMPARISON_INTERSECTS, MS_TOKEN_COMPARISON_DISJOINT, MS_TOKEN_COMPARISON_TOUCHES, MS_TOKEN_COMPARISON_OVERLAPS, MS_TOKEN_COMPARISON_CROSSES, MS_TOKEN_COMPARISON_WITHIN, MS_TOKEN_COMPARISON_CONTAINS,
     MS_TOKEN_COMPARISON_BEYOND, MS_TOKEN_COMPARISON_DWITHIN
   };
   enum MS_TOKEN_FUNCTION_ENUM {
-    MS_TOKEN_FUNCTION_LENGTH=140, MS_TOKEN_FUNCTION_TOSTRING, MS_TOKEN_FUNCTION_COMMIFY, MS_TOKEN_FUNCTION_AREA, MS_TOKEN_FUNCTION_ROUND, MS_TOKEN_FUNCTION_FROMTEXT,
+    MS_TOKEN_FUNCTION_LENGTH=340, MS_TOKEN_FUNCTION_TOSTRING, MS_TOKEN_FUNCTION_COMMIFY, MS_TOKEN_FUNCTION_AREA, MS_TOKEN_FUNCTION_ROUND, MS_TOKEN_FUNCTION_FROMTEXT,
     MS_TOKEN_FUNCTION_BUFFER, MS_TOKEN_FUNCTION_DIFFERENCE
   };
-  enum MS_TOKEN_BINDING_ENUM { MS_TOKEN_BINDING_DOUBLE=150, MS_TOKEN_BINDING_INTEGER, MS_TOKEN_BINDING_STRING, MS_TOKEN_BINDING_TIME, MS_TOKEN_BINDING_SHAPE };
+  enum MS_TOKEN_BINDING_ENUM { MS_TOKEN_BINDING_DOUBLE=350, MS_TOKEN_BINDING_INTEGER, MS_TOKEN_BINDING_STRING, MS_TOKEN_BINDING_TIME, MS_TOKEN_BINDING_SHAPE };
   enum MS_PARSE_TYPE_ENUM { MS_PARSE_TYPE_BOOLEAN, MS_PARSE_TYPE_STRING, MS_PARSE_TYPE_SHAPE };

 #ifndef SWIG

@sdlime I wouldn't be surprised there are side-effects to this, can you check ?

@sdlime
Copy link
Member

sdlime commented Aug 9, 2012

Really, really good catch. I can't see where the patch should cause new problems. I'll check to make sure the 300-360 range is unused. To be clear you're proposing only the change to mapserver.h and not a change to the lexer, correct?

Steve

@tbonfort
Copy link
Member

@sdlime what do you mean by "a change to the lexer"? If you're referring to the initial patch to maplexer.l I proposed, then no, it should not be applied.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants