Extended Static-Images Endpoint #619

benedikt-brandtner-bikemap · 2022-09-29T11:28:06Z

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 :)

mnutt

Thank you for this contribution! I'll be looking through it more thoroughly but wanted to leave a few initial comments.

mnutt · 2022-09-30T20:24:54Z

Dockerfile_test

+  && apt-get clean \
+  && rm -rf /var/lib/apt/lists/*
+
+RUN curl http://archive.ubuntu.com/ubuntu/pool/main/libj/libjpeg-turbo/libjpeg-turbo8_2.0.3-0ubuntu1_amd64.deb --output libjpeg-turbo8_2.0.3-0ubuntu1_amd64.deb


Is there a reason we need to download this manually vs apt-getting it? Is it a versioning thing?

It seems like mablibre-gl depends on a version of libjpeg-turbo with legacy-capabilities enabled for version 8 which doesn't seem to be available in the current apt repository and also a libicu version which is not in the repository.

But for Dockerfile_test i just copied the packages from Dockerfile

Have removed the commit which edited Dockerfile_test to resolve conflicts

mnutt · 2022-09-30T20:27:09Z

docs/endpoints.rst

+
+        * e.g. ``0.5`` - Scales the image to half it's original size
+
+      * ``offset`` - Image offset as positive or negative pixel value in format ``[offsetX],[offsetY]``


I'm not opposed to this by any means, but have you considered just requiring that all markers be centered both horizontally and vertically on the coordinates? IMO it makes it really easy to get placement right when designing icons, and works really well with scaling, etc.

Maybe I don't understand the question, but the markers are already centered above the selected location https://github.com/maptiler/tileserver-gl/pull/619/files#diff-e18e0ca2c1587af2fb3550206da0889f4297c04bf4e3a1d1ee09ca46041a3c3dR51 and https://github.com/maptiler/tileserver-gl/pull/619/files#diff-51ea374a3960babfb87e15ca326fc54b011b08f981c33da61d174022970a5ce6R286
I guess it would be nice to have a anchor parameter similar to https://documentation.maptiler.com/hc/en-us/articles/360020805897-Static-maps-for-your-web#Staticmapsforyourweb-Addingmarkers but idk if that should be in the scope of this PR

mnutt · 2022-09-30T20:29:50Z

src/serve_rendered.js

+    // Check if icon is served via http otherwise marker icons are expected to
+    // be provided as filepaths relative to configured icon path
+    if (!(iconURI.startsWith('http://') || iconURI.startsWith('https://'))) {
+      iconURI = path.resolve(options.paths.icons, iconURI);


I believe this would let someone specify a relative path that broke out of the icons dir and could pull from anywhere in the system. Maybe extract a function that checks that iconURI.startsWith(options.paths.icons)?

I think we should definitely try to sanitize the input here. I would like to propose something like this: acalcutt@64cad7d

Not sure if we might need to be stricter and catch even more?

have added the path sanitation with sanitize-filename

mnutt · 2022-09-30T20:32:00Z

src/serve_rendered.js

+    if (!(iconURI.startsWith('http://') || iconURI.startsWith('https://'))) {
+      iconURI = path.resolve(options.paths.icons, iconURI);
+      // Ensure icon exists at provided path
+      if (!fs.existsSync(iconURI)) {


We likely want to see if we can somehow avoid existsSync since it's in the request path. One way would be on startup to just synchronously pull a list of images in the icons dir, put them in an object, and retrieve their full paths here. It'd also solve the above issue around limiting the paths to the icons dir.

all icons are now loaded into a settings object on server startup and the provided icon is checked against this list

boldtrn

Thank you very much for this great PR @benedikt-brandtner-bikemap. The features are really awesome 👍 .

I personally think that we should add a bit of security logic to this PR. Maybe we could make requesting external images configureable. For example if this is a public server, a malicious user might host a https:// image somewhere and get the server to download this. At the very least it might be possible to force the server to download huge files and essentially DDOS the server.

The same applies to the file name, in theory a malicious user could request some sensitive files and draw them on the map or in the worst case might even run some code injection.

WDYT?

mnutt · 2022-10-03T02:55:04Z

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.

acalcutt · 2022-10-03T13:01:33Z

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

benedikt-brandtner-bikemap · 2022-10-04T12:30:32Z

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

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

acalcutt · 2022-10-04T13:32:33Z

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

benedikt-brandtner-bikemap · 2022-10-04T15:00:38Z

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

Have implemented the following fixes to address the mentioned issues:

Loading of remote images must now be allowed via the configuration parameter allowRemoteMarkerIcons
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)
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.

…hing of remote marker icons only when option is set to true; asynchronously load all available icons in a settings object on server startup; replaced fs.existsSync() call in serve_rendered when drawing marker icons with a check against available icons settings object;

benedikt-brandtner-bikemap · 2022-10-05T18:12:02Z

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

boldtrn · 2022-10-06T13:23:41Z

src/server.js

+    return files.flat();
+  }
+
+  // Load all available icons into a settings object


Great idea 👍

Thanks, did the implementation but credit goes to @mnutt fir the idea!

boldtrn · 2022-10-06T13:27:39Z

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 👍

acalcutt · 2022-10-07T00:13:48Z

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.

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

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

acalcutt · 2022-10-07T02:21:02Z

How do you use linecap?

benedikt-brandtner-bikemap · 2022-10-07T11:15:36Z

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

added linejoin parameter to staticmaps endpoint;

benedikt-brandtner-bikemap · 2022-10-07T11:28:27Z

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

acalcutt · 2022-10-07T12:56:25Z

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

benedikt-brandtner-bikemap · 2022-10-07T15:41:58Z

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

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

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

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

acalcutt · 2022-10-07T16:10:12Z

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

acalcutt · 2022-10-08T14:51:51Z

@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.

boldtrn · 2022-10-08T17:20:48Z

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

acalcutt · 2022-10-15T16:02:42Z

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 · 2022-10-15T16:17:51Z

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

acalcutt · 2022-10-16T13:22:14Z

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.

mnutt · 2022-10-16T13:43:40Z

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.

acalcutt · 2022-10-16T15:13:24Z

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.

benedikt-brandtner-bikemap · 2022-10-19T15:11:51Z

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

Is there anything I can do?

greets

acalcutt · 2022-10-20T12:28:35Z

What did you think of #619 (comment) , adding a default icon path and maybe a default, open license, icon.

acalcutt

Looks good to me. I still would like to see a default marker included, just so this can be used out of the box with the built in styles (like when just loading a mbtiles file with no config). then we could also use that default marker name in the documentation.

benedikt-brandtner-bikemap · 2022-10-27T22:50:13Z

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

acalcutt · 2022-10-28T02:55:34Z

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

acalcutt · 2022-10-28T03:33:43Z

EDIT: nevermind, fixed it with #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

benedikt-brandtner-bikemap · 2022-10-31T12:39:39Z

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

benedikt-brandtner-bikemap mentioned this pull request Sep 29, 2022

Add marker to static images acalcutt/tileserver-gl#4

Closed

mnutt reviewed Sep 30, 2022

View reviewed changes

boldtrn suggested changes Oct 2, 2022

View reviewed changes

benedikt-brandtner-bikemap added 2 commits October 5, 2022 19:29

extended staticmap capabilities

78ddbf5

benedikt-brandtner-bikemap force-pushed the staticmap-extensions branch from e0f1e38 to ea4c398 Compare October 5, 2022 18:10

boldtrn reviewed Oct 6, 2022

View reviewed changes

updated documentation for linecap parameter in staticmaps endpoint;

ec0ab3d

added linejoin parameter to staticmaps endpoint;

added capability of staticmap endpoint to draw multiple paths

4ba9bac

acalcutt mentioned this pull request Oct 8, 2022

Markers at static map [tileserver + MapTiler] #601

Closed

acalcutt approved these changes Oct 24, 2022

View reviewed changes

acalcutt merged commit f3f6349 into maptiler:master Oct 28, 2022

This was referenced Jan 10, 2023

feat: enable Google Polyline in Static Images endpoint 🕺 #648

Merged

Add markers to static map endpoint #427

Closed

acalcutt mentioned this pull request Sep 8, 2023

Static maps: All paths use styling of the last path #969

Closed

boonboonsiri mentioned this pull request Oct 6, 2023

Allow base 64 url as images for static maps #1007

Merged


		* e.g. ``0.5`` - Scales the image to half it's original size

		* ``offset`` - Image offset as positive or negative pixel value in format ``[offsetX],[offsetY]``

Extended Static-Images Endpoint #619

Extended Static-Images Endpoint #619

Conversation

benedikt-brandtner-bikemap commented Sep 29, 2022 • edited Loading

mnutt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benedikt-brandtner-bikemap Oct 4, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

boldtrn left a comment

Choose a reason for hiding this comment

mnutt commented Oct 3, 2022

acalcutt commented Oct 3, 2022

benedikt-brandtner-bikemap commented Oct 4, 2022

acalcutt commented Oct 4, 2022

benedikt-brandtner-bikemap commented Oct 4, 2022

benedikt-brandtner-bikemap commented Oct 5, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

boldtrn commented Oct 6, 2022

acalcutt commented Oct 7, 2022 • edited Loading

acalcutt commented Oct 7, 2022

benedikt-brandtner-bikemap commented Oct 7, 2022

benedikt-brandtner-bikemap commented Oct 7, 2022

acalcutt commented Oct 7, 2022

benedikt-brandtner-bikemap commented Oct 7, 2022 • edited Loading

acalcutt commented Oct 7, 2022

acalcutt commented Oct 8, 2022

boldtrn commented Oct 8, 2022

acalcutt commented Oct 15, 2022 • edited Loading

acalcutt commented Oct 15, 2022

acalcutt commented Oct 16, 2022

mnutt commented Oct 16, 2022

acalcutt commented Oct 16, 2022

benedikt-brandtner-bikemap commented Oct 19, 2022

acalcutt commented Oct 20, 2022 • edited Loading

acalcutt left a comment • edited Loading

Choose a reason for hiding this comment

benedikt-brandtner-bikemap commented Oct 27, 2022

acalcutt commented Oct 28, 2022

acalcutt commented Oct 28, 2022 • edited Loading

benedikt-brandtner-bikemap commented Oct 31, 2022

benedikt-brandtner-bikemap commented Sep 29, 2022 •

edited

Loading

benedikt-brandtner-bikemap Oct 4, 2022 •

edited

Loading

acalcutt commented Oct 7, 2022 •

edited

Loading

benedikt-brandtner-bikemap commented Oct 7, 2022 •

edited

Loading

acalcutt commented Oct 15, 2022 •

edited

Loading

acalcutt commented Oct 20, 2022 •

edited

Loading

acalcutt left a comment •

edited

Loading

acalcutt commented Oct 28, 2022 •

edited

Loading