Skip to content

CRS info added in WMS GetFeatureInfo requests#325

Merged
fgdrf merged 1 commit into
locationtech:masterfrom
nprigour:CRS_GetFeatureInfo_fix
Sep 9, 2019
Merged

CRS info added in WMS GetFeatureInfo requests#325
fgdrf merged 1 commit into
locationtech:masterfrom
nprigour:CRS_GetFeatureInfo_fix

Conversation

@nprigour

Copy link
Copy Markdown
Contributor

During WMS GetFeatureInfo requests CRS (for WMS version 1.3.0) or SRS (for WMS version 1.1.1) are not provided in the request URL. This may cause problem when retrieving layer information (see also #316 for a more analytical description of the problem)

Signed-off-by: Nikolaos Pringouris nprigour@gmail.com

Signed-off-by: Nikolaos Pringouris <nprigour@gmail.com>
@fgdrf

fgdrf commented Oct 23, 2018

Copy link
Copy Markdown
Contributor

Not really sure about this change, because @jodygarnett fixed getFeatureInfo Request a while ago (ec012ac) for WMS 1.3.0 requests, where this line has been commented.

@jodygarnett Any ideas why?

@nprigour

nprigour commented Oct 23, 2018

Copy link
Copy Markdown
Contributor Author

A while ago is almost 6 years back! Geotools have undergone several changes since then.
The only thing that the proposed patch does is to set the CRS parameter (for WMS version 1.3.0 compliant request) or the SRS parameter (for older WMS versions).
Currently this parameter is never sent for GetFeatureInfo requests and I do not know whether this is strictly compliant with the WMS standard. All other information i.e. BBOX (in north-east or east-north direction depending on the value of CRS) are more or less properly set in the request.
I verified behavior against geoserver 2.13 example layers and seems that behavior is correct for both WMS1.3 and WMS 1.1.1 requests. Maybe somebody can check the proposed patch against a MapServer installation to ensure that the problem is addressed

getmap.setBBox( bbox );
String srs = CRS.toSRS(bbox.getCoordinateReferenceSystem() );
//getmap.setSRS( code != null ? code : srs );
getmap.setSRS( code != null ? code : srs );

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we cleanup commented line 211 with this pull request?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think yes. I let it there just because it was from the initial committer

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fgdrf do you want to remove it?

@nprigour

nprigour commented Dec 14, 2018

Copy link
Copy Markdown
Contributor Author

Hi @fgdrf ,

How should we proceed with this PR?

@fgdrf

fgdrf commented Dec 16, 2018

Copy link
Copy Markdown
Contributor

Wondering if there is a demo server for mapserver that supports WMS 1.3 out there. I'd like to test against before merging it. I do not expect any issues but lets verify first ..

@nprigour

Copy link
Copy Markdown
Contributor Author

Try the following:

http://demo.mapserver.org/cgi-bin/wms?SERVICE=WMS&VERSION=1.1.1 (for wms 1.1.1 requests)
http://demo.mapserver.org/cgi-bin/wms?SERVICE=WMS&VERSION=1.3.0 (for wms 1.3.0 requests)
if you then load i.e. the world cities layer and try the info tool (prior to applying the PR) you will get a service exception indicating:
WMS server error. Missing required parameter SRS (or CRS) depending on the version type of the request
If the PR has been applied the getFeatureInfo request returns succesfully and information about the selected city is displayd in infoView.

@fgdrf

fgdrf commented Dec 17, 2018

Copy link
Copy Markdown
Contributor

Hmm, its working not as stable as expected. I was able to get information for Paris only once. Afterwards no information is shown in Information view. However I only tested with build applictaion and haven't debuged into getFeatureInfo Requests and Responses ..

Any ideas?

@nprigour

nprigour commented Dec 18, 2018

Copy link
Copy Markdown
Contributor Author

Hi @fgdrf,
I do not have any issues retrieving information for the various cities. It works as expected at least for version 1.1.1 requests. If you check the java console then you will see the requests sent in the following form:
http://demo.mapserver.org/cgi-bin/wms?Y=210&X=512&SERVICE=WMS&INFO_FORMAT=text/html&LAYERS=cities&FORMAT=image/png&HEIGHT=562&REQUEST=GetFeatureInfo&WIDTH=806&BBOX=-26.509857726305366,28.92384602977667,19.297259711417066,60.86379640198511&STYLES=default&SRS=EPSG:4326&QUERY_LAYERS=cities&VERSION=1.1.1 (Note the SRS=EPSG:4326)

Now if used version 1.3 then getFeatureInfo requests are also sent in the following form:
http://demo.mapserver.org/cgi-bin/wms?SERVICE=WMS&INFO_FORMAT=text/html&LAYERS=cities&CRS=EPSG:4326&FORMAT=image/png&HEIGHT=562&J=446&REQUEST=GetFeatureInfo&I=246&WIDTH=806&BBOX=-83.43244956259832,-14.396169602825854,-5.934758141128093,39.64068223199584&STYLES=default&QUERY_LAYERS=cities&VERSION=1.3.0 (Note the CRS=EPSG:4326)
However in the 1.3 version in most cases no information is displayed in info view or if displayed it corresponds to a wrong city. I assume that is because the mapserver is configured to work with version 1.1.1.
All in all, this does not prohibit us from verifying correct behavior of sent requests in both cases (if CRS or SRS are not provided an error is returned).
I am testing in windows machine via eclipse IDE.

@fgdrf

fgdrf commented Dec 18, 2018

Copy link
Copy Markdown
Contributor

I expected a reproducable behaviour for the same city (in this cas Paris). However, I checked with second URL and replaced parameter CRS with SRS or even removed it and server responsed an error. With parameter CRS it doesn't and works as expected

I going to write an unit test to verify CRS is set for GetFeatureInfo Requests if version is 1.3.0

@nprigour

nprigour commented Mar 7, 2019

Copy link
Copy Markdown
Contributor Author

Hi @fgdrf ,

Any progress on this PR?

@fgdrf

fgdrf commented Mar 7, 2019

Copy link
Copy Markdown
Contributor

I going to write an unit test to verify CRS is set for GetFeatureInfo Requests if version is 1.3.0

Hi @fgdrf ,

Any progress on this PR?

I started to write a test but its harder than expected to mock this with EasyMock. Sorry to say but I stoped a while ago (lack of time :()..

Can you help here and provide a test to verify that uDig adds CRS parameter for version 1.3.0 requests...?

@fgdrf fgdrf merged commit 90075fa into locationtech:master Sep 9, 2019
@fgdrf fgdrf added this to the 2.1.0 milestone Sep 18, 2019
@fgdrf fgdrf added the bug label Sep 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants