Allow user to set headers for XHR vector tile get

[gyllen]

In some cases I would like to set a header to authenticate a user for getting a vector tile. It needs to be set per source and authentication is often done in the header.

I added the following format to the source style.

  headers: [
    ["HEADER-NAME", "HEADER-VALUE"]
  ]

Added headers to ajax getArrayBuffer call.

Not sure about the what source format or if this is something that you want to support.

[gyllen]

I understand this is a quite controversial pull request. Is this something that you guys can think of supporting or should I go with patching this locally?

[lucaswoj]

Thanks @gyllen! My apologies that this has slipped through the cracks.

Here are the next steps towards merging this PR:

• a 👍 / 👎 from the TileJSON spec authors because this is an addition to that spec cc @tmcw @yhahn @jfirebaugh
• thorough test coverage of the new functionality
• documentation of the new functionality

[tmcw]

could this be Array.isArray(headers) instead? This code will fail if given a truthy non-array value

[mourner]

It still won't fail because i < undefined will always be false so the loop will effectively be noop.

[gyllen]

@lucaswoj Thanks for getting back to me on this. If this is something that gets a 👍 from the TileJSON guys I will update the PR accordingly.

Im not sure about the source format though @tmcw you think array is the way to go here?

[gyllen]

@lucaswoj Any news on this issue? (sorry for bugging you)

[jfirebaugh]

I think it's unlikely that we would want to add headers to TileJSON. In many cases TileJSON tiles are loaded via <img> tags which have no facility for adding headers, so it would become part of the spec that is unimplementable by a large portion of the ecosystem.

Could request customization of this sort fit into the Source extensibility framework we started to establish with #2667? (cc @anandthakker)

[lucaswoj]

Could request customization of this sort fit into the Source extensibility framework we started to establish with #2667? (cc @anandthakker)

Interesting question! I'll let @anandthakker answer. I haven't thought much about how it'd play with raster, image, and video sources. Sounds like something it should support in theory 😄

[anandthakker]

Yep, this should definitely be doable via a custom source. It will require some experimentation / hackery, because the custom source interface is still private and undocumented while it's being vetted and stabilized.

In the case of a raster source, you'd basically need a source type that mimicked raster_tile_source, but changed this line to an ajax call that adds the headers you want. (Those headers could have been specified/saved in the source's initial options.)

It's a bit more involved -- but still possible -- for vector sources, because in that case, you'd need something like the following:

my_vector_source.js:

var VectorTileSource = require('mapbox-gl/source/vector_tile_source')
module.exports = MyVectorSource // MyVectorSource inherits from gl-js's vector_tile_source.js
function MyVectorSource () { VectorTileSource.apply(this, arguments) }
MyVectorSource.prototype = inherit(VectorTileSource, {
   /* override VectorTileSource methods, changing it to include a `type: 'my-vector-source'` property in the params it's sending to the worker via dispatcher.send(...). */
}
module.exports.workerSourceURL = URL.createObjectURL(webworkify(require('./my_vector_worker'), {bare: true})

my_vector_worker.js:

var VectorTileWorkerSource = require('mapbox-gl/source/vector_tile_worker_source')
module.exports = function (self) {
   self.registerWorkerSource('my-vector-source', MyWorkerSource)
}

function MyWorkerSource (actor, style) {
   var vectorWorker = new VectorTileWorkerSource(actor, style, loadVectorData)

   this.loadTile = vectorWorker.loadTile.bind(vectorWorker)
   this.reloadTile = vectorWorker.reloadTile.bind(vectorWorker)
   /* and so on with abortTile, removeTile, redoPlacement */
}

function loadVectorData (params, callback) {
   // params.url is a particular tile url. 
   // callback should be called with (err, { tile: VectorTile object, rawTileData: binary pbf data ArrayBuffer })
}

[gyllen]

@anandthakker @lucaswoj @jfirebaugh Thanks for taking some time reviewing this. The custom source interface makes much more sense than adding a global property.

[mourner]

Custom vector source for such a simple task seems hacky... Could we expose a point of extension for our xhr methods so they could be patched?

[anandthakker]

@mourner I feel like that would very likely end up being more hacky, at least in this use case, since:

[the authentication header] needs to be set per source and authentication is often done in the header.

^ given that headers need to be different per source, it seems to me like patching it at xhr is too low-level -- would require inspecting the URL or something.

I do agree that the custom source approach feels like overkill, but that's partly because the custom source interface is still not as streamlined as it could be. See also #3051

[mourner]

@anandthakker yeah, I assumed something like:

if (url.indexOf(myAwesomeServer) >= 0) xhr.setRequestHeader('foo', 'bar');

[anandthakker]

I guess that's not too bad 😄 .

However, there's another issue with exposing xhr for patching: I don't think there's a straightforward way to expose the workers' copy of the module, which would be necessary to make this approach work with VT or GeoJSON source

[nextstopsun]

I have the same case as @gyllen : Need to set http headers for VT requests. What is the correct way to do this currently? A lot of things have changed since this discussion, but unfortunately no exposed API methods to set headers for particular VT sources.