Maps4HTML / Web-Map-Custom-Element

A custom <mapml-viewer> and <layer-> element suite
https://maps4html.org/Web-Map-Custom-Element/
Other
54 stars 15 forks source link

Identify Capability: how should it work on the client? #81

Open prushforth opened 5 years ago

prushforth commented 5 years ago

GeoServer now supports MapML responses for GetFeatureInfo. I have been working on implementing this in the client. The work is primarily being done in the MapML-Leaflet-Client JavaScript code, but the behaviour is emergent in the custom element implementation. I want to solicit community input for how it should work. Currently I've got it working similar to "Identify" in a GIS package, where when the user clicks or touches the map, the templated GetFeatureInfo request is triggered and the response is displayed as a vector outline with associated popup.
image

What is open to interpretation is how 'identify' should work on the stack of visible layers. There may be >1 layer in the stack that has a query link, so it might be reasonable to query them all, or it might be reasonable to just query the top-most layer that has a query link. I think to begin with, I will go with the top-most layer only, and as time or interest permits extend the behaviour per comments received here.

A more important question is: this is content that is potentially off another domain, with obvious security concerns. How should we address these concerns?

Another important question is: is there a better UI than a popup that could be implemented? Since we're trying to be cross-browser with this specification, I think we need to abstract the behaviour into a "conceptual" user interface that has potentially different takes on the popup/chrome.

cmhodgson commented 5 years ago

I think defaulting to identifying only the top layer is appropriate, this implies that we have a way to reorder the layers as well. The next step would be having the ability to select the active query layer from the layer list. I think the pop-up is a reasonable default handling of the query result, more advanced options would be to use an API to provide a html element (div) to put the identify results in, or to provide a function that takes the query event and results as parameters and does whatever it wants with them (eg. build its own style of popup dialog).

On the cross site security concerns, I think we are doing exactly what not to do: retrieving untrusted arbitrary xml and putting it into the page's DOM, unfiltered. There could be script tags or javascript: links embedded in the content that we should be filtering. I'm not sure but I think because of the way this is fetched it may step around some of the typical security measures in place by CSP. A good place to look for info on web security issues is OWASP, they have some very good "Cheat Sheets" here:

https://github.com/OWASP/CheatSheetSeries/tree/master/cheatsheets

Particularly relevant, I think, is this one on XSS: https://github.com/OWASP/CheatSheetSeries/blob/master/cheatsheets/DOM_based_XSS_Prevention_Cheat_Sheet.md

However there is a lot to it and is a little longer than the typical cheat sheet (good luck printing it upside down on your t-shirt!)

prushforth commented 5 years ago

Chris and I had a conversation offline. With his permission, I'm posting that conversation in this comment. I apologize if I got any of the threading wrong. I am still a bit unsure if we've agreed on anything yet, beyond limiting the query to the top-most layer for now.

Chris:

when I click on Canada it zooms to far in and too far north, I wonder if you are just zooming to the first polygon in the multipoly, and that happens to be Ellesmere island or something? Seems to be worse depending on the initial view when you hit the query, but often the bottom 1/2 of mainland Canada is panned off the bottom of the map window.

Peter

Weird because it shouldn’t zoom, only pan when you query. I’m not quite sure what it’s doing internally as I’m using Leaflet Layer.bindPopup on the vector feature.

Chris

I think I understand the issue with the Canada pan/zoom, it is the popup pushing the map down because it is tall and trying to fit above the center point or click point. This is less of an issue with a larger map area.

Chris

To me, the styling of the query output should be determined by the web app, not the map server.

Peter

That’s not how HTML works, though is it? I mean the server sends the HTML+ linked resources, and so it’s the server that determines how the content is styled, even if it’s by means of scripting.

Peter

MapML markup content should be ‘safe’, because to date we have said that there isn’t the notion of script in MapML, so the handling of MapML in context would be ‘safe’, so long as nothing unsafe was done with it. Also, if content is loaded from another domain via CORS, maybe there’s something about that the browser could automatically prevent script from executing, but still preserve the layout power of markup+CSS. I was thinking of trying to put the properties content in an iframe and specifying a feature policy that would prevent abuse (don’t know how at this point).

Chris

The thing is, HTML isn't typically reused on different sites. You typically want to be able to use the same map data on different map sites, and you want to be able to style it appropriately for the specific usage. There are potentially two different servers involved, the one with the Map data, and the one with the mapping app, and the HTML/code in the mapping app should be able to control the styling of the map data. And certainly the map data shouldn't be able to interfere with the code or styling on the rest of the map app. This is why I feel your HTML parallelism breaks down; we are trying to build something more like an IFrame than a video player. And IFrames cause a lot of trouble, from a security and management standpoint, particularly when you want to allow interaction between the stuff in the IFrame and the stuff outside of it.

Chris

Should really just be data, not HTML, in a fixed schema. This also solves the security problem because we are only take safe strings from the content of the mapml, not any actual markup.

Chris

I guess the table could just use a little bit of css, maybe some padding and margins, alternating background colors on the rows, maybe borders or at least a header underline... I guess that could be included in the mapML query output but that could be argued either way.

Peter

Yes it could be argued either way, but if MapML occupies the same ‘space’ as HTML i.e. presentational, the HTML/MapML should be ‘in charge’ of styling. What I would agree with is if the query was tagged to return GeoJSON, e.g.:

<link rel=query type=application/json+geo …>

that before the popup was presented it could fire an event and hand the content to the web page. Then the web page could, using script, transform that GeoJSON to any content it liked, or even just eat the event. What we would have to figure out is how the browser should choose between query links tagged ‘type=”application/json+geo”’ and those tagged ‘type=”text/mapml”’ or anything else, if there was > 1. Maybe the fact that the web page registered an event listener for it would be enough and every click / query would be passed through the handler first, then the handler could decide whether to eat the event or process it.

Chris

However at this point, we could just use geoJSON, there would be no need for a mapml vector data file type.

Peter

See above, if JavaScript was not enabled GeoJSON would not look too good.

Chris

If we are just copying HTML elements out of the map attribute data and into the page, it is not safe. There is all kinds of injection that can happen. I agree that once we have a native-built mapml support, the browser's CORS and CSP controls should be able to apply to the loaded mapml elements. But as long as we are emulating this using ajax, we are responsible for cleaning out unsafe stuff. And if there is a long list of unsafe stuff that you can't use in your mapml, why shouldn't we just have a standardized attribute schema?

I don't see the use for defining the presentation of the attributes ONLY on the mapml server side. Other than perhaps a very generic map viewer usage, I can't think of a situation where I've built a web map app and not wanted very specific control over how the attribution is displayed.

I withdraw my GeoJSON suggestion, we definitely want features on the map to be DOM objects, so we need an html/mapml representation. I maintain that there needs to be some structure around the attribution so that a client app that is not familiar with the details of a particular mapml file can find and display the attributes.

I know you are trying to enable the power of markup for links, etc

Peter

Yes :-).

Malvoz commented 3 years ago

https://github.com/Maps4HTML/Web-Map-Custom-Element/issues/81#issuecomment-487762282:

I think defaulting to identifying only the top layer is appropriate, this implies that we have a way to reorder the layers as well. The next step would be having the ability to select the active query layer from the layer list.

I believe the above is currently the case. OTOH a Feature Index (https://github.com/Maps4HTML/Web-Map-Custom-Element/issues/397) could allow querying any number of layers without having to re-order layers.

The ability of re-ordering layers from the layer control was implemented in https://github.com/Maps4HTML/Web-Map-Custom-Element/pull/249.

I think the pop-up is a reasonable default handling of the query result, more advanced options would be to use an API to provide a html element (div) to put the identify results in, or to provide a function that takes the query event and results as parameters and does whatever it wants with them (eg. build its own style of popup dialog).

The popup as a default is being replaced with a popup event: https://github.com/Maps4HTML/Web-Map-Custom-Element/issues/403.

On the cross site security concerns, I think we are doing exactly what not to do: retrieving untrusted arbitrary xml and putting it into the page's DOM, unfiltered. There could be script tags or javascript: links embedded in the content that we should be filtering. I'm not sure but I think because of the way this is fetched it may step around some of the typical security measures in place by CSP.

The retrieved HTML from querying layers is now sandboxed and scripts are not allowed: https://github.com/Maps4HTML/Web-Map-Custom-Element/pull/415. This is not to say there aren't security issues in other aspects of the map viewer, but that'd be separate issues.

In https://github.com/Maps4HTML/Web-Map-Custom-Element/issues/81#issuecomment-488063351 there is some concern around the server being able to determine styling. I think that's best discussed in a separate issue since this one is covering a lot of things. If there's nothing else unresolved here can we close this?

pvgenuchten commented 2 years ago

+1 for supporting alternative formats then mapml for visualising feature-info information.

But it brings up the topic how do you present the data, it would be nice if a template configuration is offered on the layer- element for balloon content markup.

<layer-properties-template>
<b>{{name}}</b>
<img src='{{image-url}}'/>
<a href="{{url}}">Read more</a>
</layer-properties-template>

Note that strictly the developer has to check in wms-getcapabilities which feature-info formats are actually supported by the wms server, some servers only have plain text and application/gml+xml (most also do support html, some have geojson)

Note that WMS allows to have multiple layers in a single request, if you trigger featureinfo on it, it will return results from mulitple layers, eg:

example.com/wms?request=getmap&layers=first,second,third&...

Featureinfo (or hover) is relevant for: