#2528 closed defect (fixed)
restore version as optional parameter for GetCapabilities
Reported by: | tomkralidis | Owned by: | tomkralidis |
---|---|---|---|
Priority: | normal | Milestone: | 5.2 release |
Component: | WMS Server | Version: | svn-trunk (development) |
Severity: | normal | Keywords: | |
Cc: | mapserverbugs, pal.kristensen@… |
Description
Email from Pål Kristensen:
I did a build of mapserver nightly this evening and noticed some changed WMS behavior compared to my last build (approx 2 months ago). The old build does reply to at GetCapabilities request that misses the version parameter with a version 1.1.1 capabilities xml-file. The new build fails withe an error message that says that the version parameter is missing. According to the WMS 1.1.1 standard version is a mandatory parameter so the new behavior is correct according to the standard. Unfortunately many of the clients around do not comply with the standard, and of course the server gets the blame.
Change History (14)
comment:1 by , 17 years ago
Cc: | added |
---|
comment:3 by , 17 years ago
comment:5 by , 17 years ago
BTW, do we not need to distinguish between version being not specified (optional) and version being specified in an invalid format? There is a legit case to produce an exception if the version format is invalid, but not if it's simply omitted.
I'm working on a fix to handle this.
comment:6 by , 17 years ago
Good point. Might want to add some tests to msautotest as well for this.
comment:7 by , 17 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
I have modified msOWSParseVersionString() to return either OWS_VERSION_NOTSET (-1) or OWS_VERSION_BADFORMAT (-2) if the version cannot be parsed, updated all calling code to use those two new #defines instead of the -1 constant, and finally updated the test that was at the root of this ticket in mapwms.c to still produce an exception if the version format is invalid, but not if it's omitted since it's optional in the GetCapabilities case.
I have not added tests to msautotests, I will leave that to you Tom.
comment:8 by , 17 years ago
Thanks for this. I've tested with:
$ ./mapserv QUERY_STRING="map=/tmp/test.map&service=WMS&version=1."
and I do not get back an exception, but I should.
Does msOWSParseVersionString need fixing?
comment:9 by , 17 years ago
Dan: here's a patch that works against stuff like "1", "1.", ".1"; not sure if you're working on this already, I will commit if you are not and this change is satisfactory.
Index: mapows.c =================================================================== --- mapows.c (revision 7411) +++ mapows.c (working copy) @@ -391,8 +391,19 @@ if (pszVersion) { int nVersion = 0; + int nValid = 0; + int i; + digits = msStringSplit(pszVersion, '.', &numDigits); - if (digits == NULL || numDigits < 2 || numDigits > 3) + + for (i=0;i<numDigits;i++) { + if (strcasecmp(digits[i], "\0") == 0) { + nValid = 1; + break; + } + } + + if (digits == NULL || numDigits < 2 || numDigits > 3 || nValid == 1) { msSetError(MS_OWSERR, "Invalid version (%s). Version must be in the "
comment:11 by , 17 years ago
Well, if we are going to be checking for an invalid format. Here's what is from OWS Common 1.1, subclause 7.3.1:
" Each OWS Implementation Specification revision shall specify a version number, which enables interacting clients and servers to agree on which version of the specification they are conforming to. A version number shall contain three non-negative integers separated by decimal points, in the form "x.y.z". The integers y and z shall not exceed 99. "
Since MapServer does not support any versions of any specifications which break these conditions.
comment:12 by , 17 years ago
WMS 1.0.0 used to be more open on the version format, that's probably why the current implementation is more relaxed:
6.1 Version Numbering and Negotiation 6.1.1 Version Number Form The specification version number contains up to three positive integers, separated by decimal points. The following forms are possible: "x", "x.y", or "x.y.z".
The current implementation treats ".1" as 0.1, and "1." as "1.0" which is an acceptable behavior to me even if it's not not strictly *enforcing* the text of the OWS common.
My vote goes to leaving things as is unless there is a CITE test that we fail because of that.
comment:13 by , 17 years ago
Yet another new (OWS Common) vs old "common approaches" issue. We can leave this for now, but at least we have this on record.
Having said this, I'll probably make a tighter msOWSCommonParseVersionString in mapowscommon.c if/when required, given the SOS support and in general on how OGC services are moving w.r.t. OWS Common.
comment:14 by , 17 years ago
Please no, we don't need a separate one.
My point is simply that being more strict than we are today in this specific case would bring no value. The current msOWSParseVersionString() makes enough checks to make sure that any reasonable input values results in a valid request response, and there is no known possibility that feeding junk through the VERSION param can crash MapServer or result in unpredictable behavior. Adding more validation would be purely useless overhead as fas as MapServer is concerned.
Let's leave it up to CITE to make tests to sTrIcTlY eNfOrCe the text of the specs. ;)
The only related change I could spot is http://trac.osgeo.org/mapserver/changeset/7352. I have tested this in branch-5.0 and it does work. Will fix/revert for trunk.