GeoServer

This should be simple enough to do by attaching a version to the current FilterKvpParser and having it use the appropriate configuration for the version. We will also have to break out a third parser for wms requests, and that begs the question of which version to use...although i should not matter much.

Current behaviour is:

• try parse with 1.0.0,
• if it fails, fall back to the old, non gtxml parser.

What about just adding:

• try parse with 1.0.0,
• if it fails, try parse with 1.1.0 parser
• if it fails, fall back to the old, non gtxml parser.

It is as ugly as it is, but also easier, does not break current behaviour, and allows me to send a 1.1 filter from the wfs 1.1 client asap

So my proposed approach seems like the way to go for 1.6.x, since we don't have per-service-and-version kvp parsers in 1.6.x, but starting in 1.7.x.
I can do as justin said for 1.7.x and trunk though.

The problem with using a concrete filter kvp parser for the given wfs version is that, since you have to feed the spring beans with the service name and service version they apply for, when you submit a request, if the request shall contain both the SERVICE and VERSION parameters in order for the proper kvp parser to be catched. Otherwise the request fails.
Ok, both service and version are mandatory, but it'll still break some old functionality. For instance, our demo requests do not contain either one or both of them.

suggestions?

My options so far are:

• break old, non standard behaviour and make version and service really mandatory
• use the same approach than for wms and a single filter parser

wms and wfs filter parsing has been split out.
wfs filter kvp parser uses two different kvp parsers depending on whether the request is 1.0 or 1.1.
Yet, for the cases where the version number is not specified in the query string, I've set up a 1.1 parser as default in applicationContext.xml

The comment above applies to 1.7.x+
1.6.x uses the single lax parser since there's no per service/version kvp parsers on the 1.6.x series

Gabriel, it seems the issue is still there in 1.7.1. Did you add a test?

See http://jira.codehaus.org/browse/GEOS-2559

For Gabriel: you stated the separated filters for 1.0 and 1.1 were in 1.7.x and trunk, whilst in fact they are only only trunk. And even there, strangely enough, the code falls back on the old parser. Now, for 1.0 it may make sense (the old parser was more permissive, it was parse what the new parser cannot) but for 1.1, it does not make sense at all, as it would end up reporting back an error about an invalid 1.0 filter.
On 1.7.x the parser tries to parse an Filter 1.0 filter, so webb is right, the bug has never been fixed.
Was there any reason for not fixing this on 1.7.x as well? Maybe it made a cite test fail? Someone remembers?

For webb: you filter is a syntactically invalid 1.1 Filter, that's why you get the error message you get (see also my explanation about about what's happening for invalid filters on 1.7.x). There are two errors:

• the element for coordinates is gml:posList, not gml:PosList
• the coordinates are written like in gml 2, but Filter 1.1 uses GML3, the coordinates have to be separated by spaces (even between x and y). Have a look at sample GetFeature 1.1 request outputs for an example.
  Anyways, even fixing your request won't lead you to a good result as the parser is in fact broken.

about not being applied to 1.7.x yet:
Problem is:

• even if the spec states the version param as mandatory, we treat it as optional. Dispatcher assigns 1.1 is missing.
• on deciding whether to use the 1.0 or 1.1 filter kvp parser, we can't know cause Dispatcher assigns the version after
  the fact (ie, after having processed the kvp parsers) if my memory serves me well.

I remember having talked about this with jdeolive. My opinion is that if version is mandatory, we should keep it mandatory.

So you did not fix it on 1.7.x because it was not working if the version was not set? But like this it's not working for all 1.1 requests, flat.
Also, since the thing was fixed on trunk only:

• is the trunk dispatcher behaving differently?
• why was this marked as fixed for 1.7.0-rc1?

Looking into how kvp parsing works, I see in KvpUtils.parse(Map), line 446, that if the version is not set in the request, only parsers that do not specify a version will be used.
So, I see two roads:

• we declare three kvp parsers, the 1.0 specific, the 1.1 specific, and again the 1.1, but without specifying a version for it. That seems to effectively make it the "default" parser
• we enfore version as a parameter, and refuse to answer if missing (conforming to cite, with some backwards compat issues thought).

Stopping "progress" as this needs more feedback and has been "in progress" forever