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

Avoid jsonp #5040

Closed
tomhughes opened this issue May 16, 2018 · 9 comments
Closed

Avoid jsonp #5040

tomhughes opened this issue May 16, 2018 · 9 comments
Labels
chore Improvements to the iD development experience or codebase field An issue with a field in the user interface

Comments

@tomhughes
Copy link
Member

tomhughes commented May 16, 2018

Is there a reason why the wikipedia and wikidata API access uses jsonp rather than just fetching the JSON data with an XHR request?

The reason I ask is that I'm hoping to enable Content-Security-Policy enforcement for www.openstreetmap.org soon and as it stands I've had to enable script evaluation for *.wikipedia.org and www.wikidata.org but it's not clear that is really necessary.

@bhousel
Copy link
Member

bhousel commented May 16, 2018

Hmm, I don't think there is a reason - @1ec5 can we just use d3.json to fetch these?

@bhousel bhousel added field An issue with a field in the user interface chore Improvements to the iD development experience or codebase labels May 16, 2018
@mmd-osm
Copy link
Contributor

mmd-osm commented May 16, 2018

Looks like there's some prior discussion on this topic: #2680 (comment)

@pnorman
Copy link
Contributor

pnorman commented May 17, 2018

We should get CORS enabled on the specific read-only endpoints we're using, or come to the conclusion that WMF doesn't want their services used by external sites and do something from data dumps.

@1ec5
Copy link
Collaborator

1ec5 commented May 17, 2018

can we just use d3.json to fetch these?

Sure, sounds good to me. (For the Wikidata field, I just followed the example of the preexisting Wikipedia field, which also used JSONP.)

We should get CORS enabled on the specific read-only endpoints we're using, or come to the conclusion that WMF doesn't want their services used by external sites and do something from data dumps.

The query and wbgetentities actions are officially documented parts of MediaWiki’s public API, so I’m pretty sure the intention was for them to be used by external sites. (query has been used by bots and desktop editors for well over a decade.) As far as I can tell from Phabricator tickets, Wikimedia does enable CORS for requests coming from specific sites, but iD can be hosted literally anywhere.

@bhousel
Copy link
Member

bhousel commented May 29, 2018

Hmm @tomhughes I wonder if this would be an issue for #5050 - it also seems to use JSONP.

@tomhughes
Copy link
Member Author

Well that will certainly need a new CSP rule, which is fine. Allowing XHR is preferable to to JSONP though if it's possible.

@bhousel bhousel changed the title Wikipedia and wikidata API access using jsonp Avoid jsonp Jun 7, 2018
@bhousel
Copy link
Member

bhousel commented Jun 7, 2018

There are a bunch of other uses of JSONP in the iD codebase that we should try to replace with XHR requests if possible.. I'm updating this issue to list them here:

  • Wikipedia
  • Wikidata
  • Esri Imagery Metadata
  • Bing Streetside

@Abbe98
Copy link
Contributor

Abbe98 commented Jul 3, 2018

For most uses limited to reading data from Mediawiki APIs adding CORS should be as easy as adding the origin parameter with the value *.

Example:
https://www.wikidata.org/w/api.php?action=wbgetentities&ids=Q24871&format=json&origin=*

@bhousel
Copy link
Member

bhousel commented Jul 5, 2018

This was mostly done in #5123 - we can't replace the Bing Streetside calls right now, but will keep it in mind for the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Improvements to the iD development experience or codebase field An issue with a field in the user interface
Projects
None yet
Development

No branches or pull requests

6 participants