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

JSONP #3

Merged
1 commit merged into from
Mar 15, 2011
Merged

JSONP #3

1 commit merged into from
Mar 15, 2011

Conversation

supermethod
Copy link

Hi Kevin

I've been using your plugin for a few months now very successfully (donation on its way!) so thanks for all your hard work. However I needed the plugin to return data in JSONP format so I made some simple modifications to wrap the output in a callback function passed in the initial request.

Feel free to reject but thought it might be of use to others needing JSONP access to their cakePHP app. Any feedback appreciated.

Best wishes
Chris

@kvz
Copy link
Owner

kvz commented Mar 15, 2011

Cool, I've merged in your patch, thanks.
If you have a code sample of how you're currently using this I can also add the docs for it.

@supermethod
Copy link
Author

Thanks for this, awesome!

No extra PHP code or configuration is required on the server side with this patch, just supply either the parameter callback or jsoncallback to the json url provided by your plugin and the output will be wrapped in mycallback as a function.

I'm using this for a Sencha Touch app where I need access cross domain - but I think maybe a jquery example might be more illustrative - jquery's getjSON will automatically switch to a JSONP call if ?callback=? is present in the url for example:

jQuery.getJSON("http://www.yourdomain.com/products/product.json?callback=?", 
function(data) {
    alert("Product: " + data.product.name + ", Price: " + data.product.price);
});

Jquery replaces the question mark in callback=? with its own function name generated at the time of the request (and removed after use).

Good explanations of typical JSONP usage here (jquery):
http://remysharp.com/2007/10/08/what-is-jsonp/
http://www.ibm.com/developerworks/library/wa-aj-jsonp1/

Hope that helps
Chris

@kvz
Copy link
Owner

kvz commented Mar 17, 2011

Thanks a lot Chris!

However, I think your example works equally well without the JSONP patch by just using jQuery's success handler? E.g.

jQuery.getJSON("http://www.yourdomain.com/products/product.json", 
function(data) {
    alert("Product: " + data.product.name + ", Price: " + data.product.price);
});

I've added another example in the docs.
Thanks again, and let me know if you think this is correct!

@supermethod
Copy link
Author

Hi Kevin, thanks for adding the documentation, looks great! Yes - while the JSONP patch isn't necessary to read json in jquery, its my understanding the example you have above will only work if the javascript is executed on the same domain as the json feed its calling?

If the json url is on a different domain, ?callback=? must be added to the url so jquery knows to use the script method rather than a standard ajax call (http://api.jquery.com/jQuery.getJSON/#jsonp) - and the server must return the json wrapped in the passed in callback name which is where the patch comes in.

So this patch is only really useful for those needing to access their json in cross-domain or for public api access via javascript.

Also, just thought about something else - after reading this: http://www.metaltoad.com/blog/using-jsonp-safely I think the callback parameter needs to be sanitized. I can submit another patch if you want?

Best
Chris

@kvz
Copy link
Owner

kvz commented Mar 17, 2011

Ah, thanks a lot for clarifying. I've never used JSONP before.
Also thanks for pointing out the XSS vulerability, I've just committed a patch for it. Let me know if this works for you.

@supermethod
Copy link
Author

That's cool, thanks for sorting. Again thanks your hard work on this plugin it's made my life a lot easier... also haven't forgot about donating will do so asap. Cheers

@kvz
Copy link
Owner

kvz commented Mar 18, 2011

Thanks man, appreciated : )

@ghost ghost assigned kvz Aug 10, 2012
This pull request was closed.
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.

2 participants