mapbox / mapbox-studio-classic

https://www.mapbox.com/mapbox-studio/
BSD 3-Clause "New" or "Revised" License
1.14k stars 229 forks source link

Middleware drops host part of source URI #1288

Open zerebubuth opened 9 years ago

zerebubuth commented 9 years ago

In lib/middleware.js, the function normalizePaths drops the host portion of the URI for the id and source entries in the body object. This means that if your source is a remote TileJSON URI on a different host, then any attempt to save the style results in an error saying "Invalid URL".

For the specific case that I am experiencing, the patch below fixes it. However, I assume normalizePaths was dropping the host for a reason, so perhaps this is not an appropriate fix for everyone (refs #1278), but I'd like to try and find a fix which allows us to use remote TileJSON sources.

diff --git a/lib/middleware.js b/lib/middleware.js
index c723d63..59438c2 100644
--- a/lib/middleware.js
+++ b/lib/middleware.js
@@ -22,7 +22,7 @@ middleware.normalizePaths = function(req, res, next) {
     ['id','source'].forEach(function(key) {
         if (!req.body || !req.body[key]) return;
         var uri = tm.parse(req.body[key]);
-        req.body[key] = (uri.protocol ? uri.protocol + '//' : '') + uri.dirname;
+        req.body[key] = (uri.protocol ? uri.protocol + '//' : '') + uri.host + uri.dirname;
     });
     next();
 };
yhahn commented 9 years ago

@zerebubuth sorry about that -- do you want to roll a branch/PR for this change? I added a bunch of testcases to cover the issues we were seeing (read: Windows paths) so if your change handles those correctly then I think all that would be left to do would be to add some tests for sample hostnames that you expect to work with your changes.

https://github.com/mapbox/mapbox-studio/blob/mb-pages/test/middleware.test.js#L44-L100

zerebubuth commented 9 years ago

Sounds good. I'll prepare a PR.

jbrwn commented 9 years ago

@zerebubuth @yhahn I am running into this issue as well in mapbox studio 0.2.7 OSX. I ran into this trying to use non mapbox remote sources but even using a mapbox tilejson uri fails:

http://a.tiles.mapbox.com/v3/joelsbrown1.ffb527f1.json

screen shot 2015-08-07 at 10 32 50 am

I realize that this is a fringe use case for mapbox studio but it would great if this worked. Is there any chance this will be fixed in a future release? Are there any workarounds for getting this to work in version 0.2.7?