`msCopyMap()` does not copy `VALIDATION` blocks #4596

Closed
homme opened this Issue Feb 27, 2013 · 12 comments

Projects

None yet

4 participants

@homme
Contributor
homme commented Feb 27, 2013

It does not appear that VALIDATION blocks are copied by msCopyMap() at any of the places they can occur (WEB, LAYER, CLASS). Implementing a msCopyValidation() function and calling it from msCopyLayer(), msCopyWeb() and msCopyClass() would solve this.

@tbonfort tbonfort was assigned Feb 27, 2013
@tbonfort
Member

@homme I'll let you close this one once you've confirmed the previous commit is ok.

@homme
Contributor
homme commented Mar 4, 2013

@tbonfort Thanks. VALIDATION blocks in WEB and LAYER seem to copy fine now, but not CLASS.

I've had a quick look and this seems to be because CLASS level validation blocks aren't being honoured properly. The following snippet is based on tests/test.map and is used to do some runtime substitution using mapserv CGI:

CLASS
  NAME "0"
  EXPRESSION ("[FNAME]" eq "fname%")
  METADATA
    "key1" "value1"
    "key2" "value2"
    "key3" "value3"
    "key4" "value4"
  END
  STYLE
    COLOR 255 153 102
    OUTLINECOLOR 0 0 204
    #SYMBOL 1
    #SIZE 2
  END
  VALIDATION
     "fname"     "^[a-zA-Z ]+$"
  END
END

I put a msSaveMap(mapserv->map, "/tmp/out.map") call in after msCGILoadMap(mapserv) in mapserv.c (using master branch). Looking at /tmp/out.map shows the class as above but minus the validation block. So either msSaveMap, the runtime substitution or the map loading is buggy (or a combination thereof!).

@tbonfort
Member
tbonfort commented Mar 4, 2013

@homme, I'll let you try again with this new commit...

@homme
Contributor
homme commented Mar 5, 2013

@tbonfort We're making progress - the validation block is now written to the mapfile :) However, this appears to have been a separate bug as the validation and runtime substitution itself still doesn't appear to be working from within a CLASS. The following is the standalone mapfile I'm using to test:

MAP
    NAME "Testing"
    EXTENT -0.5 50.977222 0.5 51.977222
    IMAGETYPE PNG
    IMAGECOLOR 255 255 255
    STATUS ON
    SIZE 200 200

    PROJECTION
        "init=epsg:4326"
    END

    LAYER
        NAME "POLYGON"
        TYPE POLYGON
        PROJECTION
            "init=epsg:4326"
        END
        STATUS ON # DEFAULT
        DATA "polygon"
        PROCESSING "ITEMS=FNAME"
        FEATURE
            WKT "POLYGON ((-0.25 51.227222,-0.25 51.727222,0.25 51.727222,0.25 51.227222,-0.25 51.227222))"
            ITEMS "A Polygon"
        END

        #METADATA
        #    "fname_validation_pattern" "^[a-zA-Z ]+$" # works
        #END

        #VALIDATION
        #    "fname"     "^[a-zA-Z ]+$" # works
        #END

        CLASSITEM "FNAME"
        CLASS
            NAME "0"
            EXPRESSION ("[FNAME]" eq "%fname%")

            #METADATA
            #    "fname_validation_pattern" "^[a-zA-Z ]+$" # doesn't work
            #END

            VALIDATION
                "fname"     "^[a-zA-Z ]+$" # doesn't work
            END

            STYLE
                COLOR 255 153 102
                OUTLINECOLOR 0 0 204
            END
        END
    END
END

An request to this mapfile along the following lines...

http://localhost/cgi-bin/mapserv?map=/tmp/test.map&mode=map&layer=POLYGON&fname=A+Polygon

...should yield the following image:

test

but instead fails to draw the orange polygon.

@tbonfort
Member
tbonfort commented Mar 5, 2013

ok, the class->validation block is being ignored when checking for validation patterns. cc/ @sdlime . To fix, in msApplySubstitutions() in mapfile.c , we probably need a nested loop iterating through the layer classes to apply the substitutions on a class-by-class basis instead of globally for all the layer, and layerSubstituteString() needs to be split up into layerSubstituteString() and classSubstituteString()

@homme
Contributor
homme commented Mar 5, 2013

This also raises the issue of validation precedence assuming the same substitution parameter name appears in a combination of WEB, LAYER and CLASS validation blocks but with different patterns. At the moment it looks like LAYER validations takes precedence over those in WEB and it seems to make sense that CLASS validations take precedence over both. I am happy to add this to the documentation (probably at http://mapserver.org/cgi/runsub.html#validation).

@tbonfort
Member
tbonfort commented Mar 5, 2013

This precedence makes sense. If you also want to provide a patch to fix class->validation, please be our guest :-)
On a side note, I also was wondering if we could stop supporting metadata level validation keys, as they have been deprecated since RFC44 (i.e. 5 years ago)

@homme
Contributor
homme commented Mar 5, 2013

I'll see what I can do. It also looks like we would also need to make msApplyDefaultSubstitutions() aware of class validations, which would have the same precedence issues.

@tbonfort
Member
tbonfort commented Mar 5, 2013

The default substitutions don't need any validating, as they are provided in the mapfile and cannot be overridden by url

@dmorissette
Contributor

+1 to remove validation metadata support (deprecated in RFC-44) as long as there is a clear note in the 6.4 migration guide about this. That removal would be worth a separate ticket I think.

@sdlime
Member
sdlime commented Mar 5, 2013

Also +1 on removing validation metadata support. Note that validation is used for more than run-time subs. It also controls URL-based access to parameters (kinda like the old DATAPATTERN and TEMPLATEPATTERN options). You can also use the validation block to mark something as immutable. Probably need to verify that works with class validation as well.

@homme
Contributor
homme commented Mar 11, 2013

@sdlime Yes, it looks like class validation keys used are expression, group, keyimage, template, text and title. I can't see any tests for these keys but I don't believe the changes in my pull request affect any of them.

@tbonfort tbonfort added a commit that closed this issue Mar 12, 2013
@homme @tbonfort homme + tbonfort Enable runtime substitutions using `class` level `validation` blocks (#…
…4600)

 As well as now honouring
class level validation blocks a validation hierarchy is implemented
across `web`, `layer` and `class` level validation blocks.  This
hierarchy *only* takes effect when identical validation keys appear in
`web`, `layer` or `class` such that keys in more specialised blocks
override those in more generalised blocks. i.e. `class` overrides
`layer` which overrides `web`.

In conjunction with the above, default class level substitutions are
also enabled and also implement the precedence rules above. i.e. a
default substitution in `class` overrides `layer` which overrides
`web`.

Ensure validation *always* occurs when a substitution is requested
Fixes #4596
Fixes #4600
4e79097
@tbonfort tbonfort closed this in 4e79097 Mar 12, 2013
@homme homme added a commit to homme/docs that referenced this issue Mar 18, 2013
@homme homme Update Runtime Substitution documentation
This documents bugfixes and enhancements introduced when addressing
mapserver/mapserver#4596.
354ecbe
@homme homme referenced this issue in mapserver/docs Mar 18, 2013
Merged

Update Runtime Substitution documentation #25

@mkofahl mkofahl pushed a commit to faegi/mapserver that referenced this issue Apr 9, 2013
@tbonfort tbonfort add validation block to copy functions (#4596) 491b92e
@mkofahl mkofahl pushed a commit to faegi/mapserver that referenced this issue Apr 9, 2013
@tbonfort tbonfort write CLASS validation to mapfile (#4596) 1b74670
@mkofahl mkofahl pushed a commit to faegi/mapserver that referenced this issue Apr 9, 2013
@homme @tbonfort homme + tbonfort Enable runtime substitutions using `class` level `validation` blocks (#…
…4600)

 As well as now honouring
class level validation blocks a validation hierarchy is implemented
across `web`, `layer` and `class` level validation blocks.  This
hierarchy *only* takes effect when identical validation keys appear in
`web`, `layer` or `class` such that keys in more specialised blocks
override those in more generalised blocks. i.e. `class` overrides
`layer` which overrides `web`.

In conjunction with the above, default class level substitutions are
also enabled and also implement the precedence rules above. i.e. a
default substitution in `class` overrides `layer` which overrides
`web`.

Ensure validation *always* occurs when a substitution is requested
Fixes #4596
Fixes #4600
43b3fe8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment