tileserver-gl icon indicating copy to clipboard operation
tileserver-gl copied to clipboard

Extended Static-Images Endpoint

Open benedikt-brandtner-bikemap opened this issue 2 years ago • 16 comments

I have Extended the Static-Images Endpoint with the following capabilities:

  • Optional border with configurable color and width for paths via the border and borderwidth parameters
  • Optional markers with the following configurable options:
    • Icon provided locally via the icons path or as a hyperlink (enabled via allowRemoteMarkerIcons setting)
    • Scale of the image relative to its original size
    • Offset in pixels
  • Linecap of the path https://developer.mozilla.org/en-US/docs/Web/API/CanvasRenderingContext2D/lineCap
  • Linejoin of the path https://developer.mozilla.org/en-US/docs/Web/API/CanvasRenderingContext2D/lineJoin
  • Capability to provide multiple paths
  • Maximum zoom level if autofit is selected when providing a path

Additional Changes:

  • In order to keep performance drawing of elements has been refactored to be asynchronous
  • Refactored a lot of helpers in serve_rendered.js to be more modular and reusable

Please let me know if this would be interesting to you.

Cheers :)

There's also the tricky concept of SSRF--allowing a server to take instruction to download any URL means it can fetch internal resources that the server has access to (ec2 instances often have access to sensitive metadata on localhost, etc) and it's not very easy to set up rules to prevent it.

Realistically, siphoning internal resources through embedded markers is probably pretty difficult but would still potentially allow someone to prod around. I think it'd probably be best to leave the remote feature disabled by default, and let people enable it knowing the security implications.

mnutt avatar Oct 03 '22 02:10 mnutt

Should this use a similar syntax to maptilers other products, like https://documentation.maptiler.com/hc/en-us/articles/360020805897-Static-maps-for-your-web . Seems like they support icon urls

acalcutt avatar Oct 03 '22 13:10 acalcutt

Hey, thanks a lot for all for you responses unfortunately i got sick over the weekend and wasn't able to respond sooner :slightly_frowning_face:

Will have a look right now and push my proposed changes to address the issues asap!

Hope you are feeling better @benedikt-brandtner-bikemap . No rush here.

acalcutt avatar Oct 04 '22 13:10 acalcutt

thanks a lot, but yeah am bored anyways so this is a welcome distraction :slightly_smiling_face:

Have implemented the following fixes to address the mentioned issues:

  1. Loading of remote images must now be allowed via the configuration parameter allowRemoteMarkerIcons
  2. Locally available icons get loaded as relative paths to the icon-folder into the settings object paths.availableIcons on server startup (thanks a lot for the idea @mnutt)
  3. When providing a local icon as a marker, the path gets sanitized first and then checked against the paths.availableIcons settings object if the icon exists and is allowed to be used

I think these measures should remedy a lot of the security concerns mentioned, although you are correct as soon as the icons get fetched from a remote server we are still exposed.

Have rebased my changes ontop of current master to resolve conflicts in Dockerfile_test

I think these measures should remedy a lot of the security concerns mentioned, although you are correct as soon as the icons get fetched from a remote server we are still exposed.

Thanks a lot for the changes. At least from my side, this solves the concerns I had and I am looking forward to using this 👍

boldtrn avatar Oct 06 '22 13:10 boldtrn

I finally got a chance to set up a test of this. Seemed to work pretty well. it took me a bit to figure out the url.

I ended up with this for my test url http://192.168.0.251:8080/styles/test-style/static/auto/800x600.png?&marker=5.9,45.9|icons8-maps.svg&marker=5.7,44.9|icons8-pin-62.png|scale:0.75&marker=5,45.1|icons8-pin-67.png|scale:0.75 and got 800x600

http://192.168.0.251:8080/styles/test-style/static/auto/800x600.png?marker=7.447447,46.94797|icons8-pin-62.png|scale:0.50&maxzoom=16 800x600 (1) Compared to google

http://192.168.0.251:8080/styles/test-style/static/auto/800x600.png?path=5.9,45.8|5.9,47.8|10.5,47.8|10.5,45.8|5.9,45.8&marker=7.447447,46.94797|icons8-pin-62.png|scale:1 800x600 (2)

acalcutt avatar Oct 07 '22 00:10 acalcutt

How do you use linecap?

acalcutt avatar Oct 07 '22 02:10 acalcutt

Hey, awesome that you approve of my changes if i should add/change anything else i'll be happy to do so!

@acalcutt I use linecap in the drawing context of the path if one is provided to define the rendering-style of the start- and end-points of the line as described here implementation

Also you can find documentation for the new staticmap endpoint parameters (including markers) here

Have updated documentation for the linecap parameter and also added a linejoin parameter as well to be able to alter the rendering style of overlapping segments as described here

Based on the docs and links, shouldn't this give me squared off line endings?

http://192.168.0.251:8080/styles/test-style/static/auto/800x600.png?&path=5.9,45.3|5.7,45.2&linecap=square 800x600 (3)

acalcutt avatar Oct 07 '22 12:10 acalcutt

Seems to work maybe you just cant see it because the width is too thin

default (butt): http://127.0.0.1:8080/styles/bikemap-2021-3D/static/auto/800x600.png?width=10&path=16.370079517364502,48.2174529909167|16.3616681098938,48.21362115453811 default

square: http://127.0.0.1:8080/styles/bikemap-2021-3D/static/auto/800x600.png?width=10&linecap=square&path=16.370079517364502,48.2174529909167|16.3616681098938,48.21362115453811 square

round: http://127.0.0.1:8080/styles/bikemap-2021-3D/static/auto/800x600.png?width=10&linecap=round&path=16.370079517364502,48.2174529909167|16.3616681098938,48.21362115453811 round

Also i've added the capability to draw multiple paths, which i wanted to do when i initially wrote the extensions but which came up again now.

sorry for extending the scope of this PR but it was only a minor change on top

Your right, it does work. i guess I expected more of a square at the ends. looks good.

acalcutt avatar Oct 07 '22 16:10 acalcutt

@mnutt and @boldtrn Any other things you would like to see before this is merged. it seems like most of the complaints have been fixed and it seems to work pretty well. I'd approve it and merge it in.

the only thing I wish we had was a default marker icon in our test style...but I think that would be something I would have to add to the old release zip file we use.

acalcutt avatar Oct 08 '22 14:10 acalcutt

Default Marker would be nice indeed. So far I think this PR seems nice and I'd love to start using it.

boldtrn avatar Oct 08 '22 17:10 boldtrn

I've been thinking a bit about the use case where you give tileserver a mbtiles file but do not give it a config. In that case it uses the built in style in '/node_modules/tileserver-gl-styles/' and generates it's own config, so it makes it somewhat hard to add the marker icons unless you add your own config.

I was thinking, what if we put some default marker images in 'public/resources/icons/' and in main.js add that icon path to the default generated config like this around line 125

      const styleDir = path.resolve(__dirname, '../node_modules/tileserver-gl-styles/');
      const iconDir = path.resolve(__dirname, 'public/resources/icons/');

      const config = {
        'options': {
          'paths': {
            'root': styleDir,
            'fonts': 'fonts',
            'icons': iconDir,
            'styles': 'styles',
            'mbtiles': path.dirname(mbtilesFile)
          }
        },
        'styles': {},
        'data': {}
      };

I think one side benifit of having the icons in the public folder is you could also use them in your vector maps, like https://maplibre.org/maplibre-gl-js-docs/example/add-image/

acalcutt avatar Oct 15 '22 16:10 acalcutt

maybe something CC0 licensed for a default icon, like https://uxwing.com/map-pin-icon/

acalcutt avatar Oct 15 '22 16:10 acalcutt

I was also thinking I wonder if the icons should be more like sprites so you could use a single image like https://github.com/sigma-geosistemas/Leaflet.awesome-markers/blob/master/dist/images/markers-matte.png with multiple colors in one file.

Not sure its needed to merge this, I was just wondering if that would be better. it seems like sprites are very similar to markers.

acalcutt avatar Oct 16 '22 13:10 acalcutt

I've looked over it and am good with it.

Regarding default markers: for what it's worth my old branch has a default marker that can be colorized based on a hex code:

https://github.com/mnutt/tileserver-gl/blob/master/src/markers/color.js

The 'template' uses solid red as the replacement color, and will dynamically generate PNGs of other colors.

mnutt avatar Oct 16 '22 13:10 mnutt

I've looked over it and am good with it.

Regarding default markers: for what it's worth my old branch has a default marker that can be colorized based on a hex code:

https://github.com/mnutt/tileserver-gl/blob/master/src/markers/color.js

The 'template' uses solid red as the replacement color, and will dynamically generate PNGs of other colors.

That is a really nice feature.

acalcutt avatar Oct 16 '22 15:10 acalcutt

Sorry for not beeing more active was focused on other tasks..

Is there anything I can do?

greets

What did you think of https://github.com/maptiler/tileserver-gl/pull/619#issuecomment-1279773329 , adding a default icon path and maybe a default, open license, icon.

acalcutt avatar Oct 20 '22 12:10 acalcutt

Hey, sorry for not answering sooner was again occupied with something else...

Hmm I could implement a default marker with a choosable color utilizing @mnutt lib but I think I'll only have time end of next week so maybe we merge this PR and I'll open a new one with a default marker?

Greets

that sounds fine to me. I'll merge this.

acalcutt avatar Oct 28 '22 02:10 acalcutt

EDIT: nevermind, fixed it with https://github.com/maptiler/tileserver-gl/pull/631

Hey @benedikt-brandtner-bikemap ,

I noticed this after I merged, but would it be possible to make a PR that changes

import pkg from 'canvas'; https://github.com/maptiler/tileserver-gl/blob/master/src/serve_rendered.js#L10 to import {createCanvas, Image} from 'canvas';

and remove the line const {createCanvas, Image} = pkg; https://github.com/maptiler/tileserver-gl/blob/master/src/serve_rendered.js#L25

acalcutt avatar Oct 28 '22 03:10 acalcutt

sorry just saw your comment, thanks for fixing this and merging :)!