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

Normalize JSONP #64

Closed
wants to merge 1 commit into from
Closed

Normalize JSONP #64

wants to merge 1 commit into from

Conversation

ngryman
Copy link

@ngryman ngryman commented Jun 30, 2013

@broofa
Copy link
Owner

broofa commented Jul 25, 2013

(sorry for the delay in reviewing - node-mime hasn't been on my priority list for a while)
How widely used is the 'jsonp' extension? I ask because JSONP is just JS so there's a pretty strong argument to be made that you should just be using '.js' instead.

If it's not widely adopted then it's probably not appropriate to add this to node.types. Instead, you should just use mime.define() to provide the necessary entry for your project.

Leaving this open to see if anyone else piles on here.

@ngryman
Copy link
Author

ngryman commented Jul 26, 2013

ping @alrra @AD7six @niftylettuce.

Do you know the usage of .jsonp?

@broofa
Copy link
Owner

broofa commented Jul 26, 2013

More thoughts ...

FWIW, I base my decision on whether or not merge requests like this on two things:

  1. Is there sufficient in-the-wild demand? ("are there 10+ projects that'll benefit from this?")
  2. Is there a standard that is strong enough to warrant inclusion on the basis of the standard alone.

Googling around, the answer to #1 would seem to be "no". I have yet to see a service that uses 'jsonp' as an extension for their JSONP api.

For #2, the standard, such as it is, is hosted at json-p.org. But what I find there isn't so much a standard as a collection of notes that can direct the development of a standard. Specific concerns are:

  • That comments were disabled in 2010 and the link to the github repo is broken has me thinking this site isn't getting a lot of attention.
  • It's indecisive around mime type, saying, "...the MIME-type "application/json-p" and/or "text/json-p" must be declared...". And neither of the types mentioned are the application/javascript that's been proposed here. More importantly, the reason they're not application/javascript is important - the proposal is that browsers should reject arbitrary javascript responses fed to JSON-P marked script elements.
  • No specific query param name for the callback reference is specified. There is only an example that mentions callback. While there appears to be some convention around this name there are notable variations from this on sites like Wikipedia ('json') and mootools ('jsoncallback').
  • It's not clear the security concerns were ever adequately addressed.

Anyhow, I'll ping @getify and see if I can get him to chime in here.

@getify
Copy link

getify commented Jul 26, 2013

I put up http://json-p.org several years ago when CORS really was terribly supported, because I believed we needed a bridge "standard" for cross-domain Ajax exchange, but, as stated on the site, that we could make it safer than "just javascript".

Importantly, when I noted poor support for CORS, I was talking not only about browsers, but about the popular APIs (and the popular server-side frameworks which expose APIs) and how many of them were not CORS-enabled. That seems to definitely have "corrected" itself over the last few years.

What I had hoped was that the fast-updating browsers like Chrome and Firefox could quickly add such "safer JSON-P" checks, and do so much more quickly than just waiting for all the web APIs to start supporting CORS and all the libraries relying on JSON-P to be rewritten to CORS. Alas, we just ended up waiting, and I think quite arguably we're well past the "1 - 3 years" I predicted for CORS to become ubiquitous.

Of course, all that's with a caveat that you're not trying to support "legacy" opera (12 and before) and legacy IE (7 and before). If you are such an unfortunate soul, you're stuck with JSON-P. But, those browsers aren't going to be patched with my proposed "safer" JSON-P parsing, anyway, so they're just stuck in the same boat they always have been.


I still believe in the spirit of what I proposed on json-p.org. If you really are communicating with JSON-P, I still firmly believe that you should have some parser/validator to check and make sure there's not malicious code trying to sneak through. That can still happen today, if you communicate with an untrusted API, or if a trusted API gets hacked.

The stakes are much higher if you consider the possibility of letting arbitrarily-retrieved code run in your node instance.

@getify
Copy link

getify commented Jul 26, 2013

I think the idea of supporting consuming JSON-P is still valid because there's stil a LOT of APIs out there that do so. Even if they don't need to do so, many of them provide it as an option. As I mentioned on the site, one still-not-solved use-case is unit-testing your JSON-P APIs (that is, parsing them to make sure they produce valid, correct output).

So I would suggest that you could set aside the proposal of json-p.org from a "safety" perspective, but still derive benefit from the idea of it as simply a parsing standard, and that benefit will survive for a very, very long time.

For instance, you could say for the mime-type "text/json-p" (or "application/json-p"), the standard parsing behavior is to remove the surrounding caller code entirely, and retrieve ONLY the JSON part, and return that, thereby ending around all of the security concerns raised with letting actual JSON-P run in your host environment.

That sort of approach, if someone were to do that in their code, makes the "utility" of a standard JSON-P mime-type clearer.

There's plenty of precedent for similar ideas. Most of the client-side frameworks have built-in "json" support, which means that if you get JSON back, they automatically run it through JSON.parse() for you, and hand you the object, not the code.


I am not aware of anyone using ".jsonp" as a file extension, but as the purpose of node-mime is to provide a way to recognize a mime-type when all you have is a file extension, that seems like a perfectly legitimate and safe option to include, for those who do care about handling JSON-P.

@getify
Copy link

getify commented Jul 26, 2013

After looking at the pull request, one final note to clarify: I personally don't see any utility in mapping ".jsonp" to plain old javascript mime-type. If some service is going to the trouble to expose something with that file-extension, they're definitely producing actual JSON-P and not just regular javascript code, and so the content _should be treated as JSON-P, not as JS_.

So if you're going to map ".jsonp" to any mime-type, it should definitely be different from "application/javascript", IMO. Either of the two that I "proposed" are perfectly reasonable options.

@broofa
Copy link
Owner

broofa commented Jul 26, 2013

@getify - thanks for your input on this. Given that there doesn't seem to be any convention around JSONP mime types or extensions, and that application/javascript is arguably exactly the /wrong/ type to use, I'm going to close this out for now. If/when the situation changes and there's better adoption around the proposed practices we can reconsider.

@broofa broofa closed this Jul 26, 2013
@ngryman
Copy link
Author

ngryman commented Jul 26, 2013

@getify - thanks a lot for your comprehensive input.

I tend to be agree with @broofa to not propagate usage of an extension that does not really make sense.
I will request a removal of this in other @h5bp's server config.

alrra added a commit to h5bp/server-configs-apache that referenced this pull request Jul 27, 2013
Remove `.jsonp` as there isn't any convention around the JSONP MIME-type
or the `.jsonp` extension. Also, it seems that, `application/javascript`
is arguably the wrong type to use for JSONP.

Ref: broofa/mime#64
alrra added a commit to h5bp/server-configs-nginx that referenced this pull request Jul 28, 2013
Remove `.jsonp` as there isn't any convention around the JSONP MIME-type
or the `.jsonp` extension. Also, it seems that, `application/javascript`
is arguably the wrong type to use for JSONP.

Ref: broofa/mime#64
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants