Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CZML and KML need the ability to pass query parameters #5408

Closed
mramato opened this issue Jun 1, 2017 · 11 comments
Closed

CZML and KML need the ability to pass query parameters #5408

mramato opened this issue Jun 1, 2017 · 11 comments

Comments

@mramato
Copy link
Contributor

mramato commented Jun 1, 2017

CzmlDataSource and KmlDataSource have no way to specify additional parameters that need to be included in url requests. The most common use for this is query-based access tokens.

Both classes' load function (and in CZML's case the process function) should take an additional query object, off of the options object. It would be a dictionary of strings that get converted to a query string via objectToQuery and globbed onto all urls requests.

@denverpierce
Copy link
Contributor

I'd add geojson to this as well. A good use case is ArcGIS server, which can serve up conformant Geojson, but the API calls to build the queries are unwieldy in raw string form.

@hpinkos
Copy link
Contributor

hpinkos commented Jun 1, 2017

Thanks @denverpierce! Good idea

@mramato
Copy link
Contributor Author

mramato commented Jun 1, 2017

I didn't think GeoJSON would be necessary since there's only one url involved, which the developer specifies. You can use objectToQuery to easily do what you want in that case. In CZML/KML, they retrieve additional resources depending on the content of the file, and that's something we can't currently do.

I'm not against adding it for GeoJSON for completeness, it will be trivial.

@denverpierce
Copy link
Contributor

Now that I understand the use case, I see what you're saying. They are closely related, but geojson doesn't have a burning need for that functionality, it'd just be a quality of life change.

@emackey
Copy link
Contributor

emackey commented Jun 6, 2017

Random question, does the fix here only involve query parameters? Should we support systems that transmit authentication via added headers instead of query parameters?

@ottaviohartman
Copy link
Contributor

This is only for query parameters, yes.

@mramato
Copy link
Contributor Author

mramato commented Jun 6, 2017

Should we support systems that transmit authentication via added headers instead of query parameters?

Headers are not generally a good solution for authentication that involves browser-retrieved resources. For example, Cesium uses the HTML image element for retrieving markers, image tiles, textures, etc.. but you can't assign headers when setting image.src, your only choice is a query parameter. So even if we added header support, it would not help in 99% of Cesium's use cases.

Of course if we ever had a good reason for adding a headers object, we could; it's just outside the scope of this issue.

@denverpierce
Copy link
Contributor

Isn't there limited support for reusing existing header auth via TrustedServers?

@mramato
Copy link
Contributor Author

mramato commented Jun 6, 2017

Isn't there limited support for reusing existing header auth via TrustedServers?

Good point in that the Authorization header, cookies, and certs would already be added in the case of the server being in the TrustedServers list, but general header support is impossible because of the reason I listed above. I think TrustedServers is the better mechanism to enable what @emackey originally described (and already works today)

@emackey
Copy link
Contributor

emackey commented Jun 6, 2017

Thanks. Was mostly asking a hypothetical. I think we have an internal project here that normally uses headers, but can optionally use query parameters if I'm not mistaken.

Feel free to close this if you're done with the implementation. Thanks again.

@ottaviohartman
Copy link
Contributor

@emackey KML is in progress and very close to done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants