Hi, I do not see XSS as a problem for Sling here, because it is the caller's decision to include the script tag and request data from a Sling system. The caller has to decide wether to trust Sling not to include any malicious JS and only call the specified callback function, but there is no risk of a caller compromising a Sling system by the way of JSONP.
As the alternative would be to build a proxy on the caller's backend, including JSONP support would make creating mashups that work with content served by Sling easier by an order of magnitude. regards, Lars On Sun, Apr 11, 2010 at 11:39 PM, Felix Meschberger <[email protected]>wrote: > Hi, > > On 10.04.2010 01:31, Justin Edelson wrote: > > I was under the impression this couldn't be done and, in a sense, the > > Well, maybe we can limit it ? Maybe consider the Referer header and > check whether it is another virtual-host (aka /etc/map-ed host) on the > same Sling system ? > > I we cannot make it reasonable secure, I agree, that we should probably > not support it in the core Default GET Servlets. > > Maye some sample code in the samples section with big disclaimers etc. ? > > > only reason people use JSONP is to do XSS. > > Yeah, right. > > Still there is mabye benign and malign XSS ... We may want to support > benign XSS but prevent malign XSS. If we cannot make the distinction, it > is probably better to not support it at all. > > Regards > Felix > > > > > Justin > > > > On Apr 9, 2010, at 5:11 PM, Felix Meschberger <[email protected]> > > wrote: > > > >> There's just one question: how to prevent XSS for JSONP ? > >> > >> Regards > >> Felix > >> > >> On 09.04.2010 23:06, Felix Meschberger wrote: > >>> Hi, > >>> > >>> On 09.04.2010 22:45, Justin Edelson wrote: > >>>> On 4/9/10 3:58 PM, Felix Meschberger wrote: > >>>>> Hi, > >>>>> > >>>>> On 09.04.2010 21:32, Justin Edelson wrote: > >>>>>> Hmmm. You're right - I conflated extension with suffix. > >>>>>> > >>>>>> On 4/9/10 3:10 PM, Luca Masini wrote: > >>>>>>> Great, I never considered RequestDispatcherOptions and I think > >>>>>>> it's a > >>>>>>> good idea. > >>>>>>> > >>>>>>> But with replaceSuffix I go into an infinity cycle, because the > >>>>>>> servlet > >>>>>>> is choosen by the "jsonp" extension and not as a suffix: > >>>>>>> > >>>>>>> req.getRequestDispatcher(req.getResource().getPath(), new > >>>>>>> RequestDispatcherOptions("replaceSuffix=json")).include(req, > >>>>>>> resp); > >>>>>>> > >>>>>>> > >>>>>>> What is wrong with me ??? > >>>>>>> > >>>>>>> Also, how can I associate a script like the one you showed to an > >>>>>>> extension ?? > >>>>>> Create it at /apps/sling/servlet/default/jsonp.esp > >>>>>> > >>>>>> It doesn't look like there's a way to override the extension via > >>>>>> RequestDispatcherOptions. I guess what you can do is just hack > >>>>>> the last > >>>>>> letter off the path (assuming you're using jsonp as the > >>>>>> extension). But > >>>>>> now I have a bad taste... > >>>>>> > >>>>>> I can't think of a good reason why this is the case. It seems > >>>>>> like there > >>>>>> should be a replaceExtension=json option. > >>>>> > >>>>> You could to > >>>>> > >>>>> req.getRequestDispatcher(req.getResource.getPath()+".jsonp").... > >>>> AFAICT, this won't work (and would need to be ".json" anyway) > >>>> because > >>>> you lose all the selectors. i.e. > >>> > >>> Sure, this was just a no-brainer example ;-) > >>> > >>>> > >>>> /foo/bar/res.tidy.jsonp (resource path = /foo/bar) > >>>> should be a padded version of > >>>> /foo/bar/res.tidy.json > >>>> > >>>> but if you just replace tack on the extension to the resource > >>>> path, it > >>>> would be /foo/bar/res.json. > >>>> > >>>> Obviously you could recreate the selectors if you needed to, but > >>>> this is > >>>> why it seems to me that replaceExtension would be useful. I see a > >>>> typo > >>>> in what I wrote... it should be: > >>>> <%= request.getParameter("jsonp") %>(<% sling.include(resource, > >>>> "replaceExtension=json") %>) > >>>> > >>>>> > >>>>> to overwrite the request extension. > >>>>> > >>>>> But: we didn't do this by intent: Since we assume an expected > >>>>> content > >>>>> type from the request extension changing the extension for request > >>>>> processing might interfere with these expectations. > >>>> > >>>> Maybe I'm missing something, but how could it interfere? > >>>> > >>>> If I have html.esp, what is the harm in being able to include the > >>>> result > >>>> of txt.esp or json.esp? The response content type is still text/ > >>>> html. I > >>>> agree that this probably wouldn't be very common, but I don't see > >>>> the harm. > >>>> > >>>>> > >>>>> Now, what would hinder use to add support for JSONP in our JSON > >>>>> servlets > >>>>> ? We could, for example add JSONP servlets to the Default GET > >>>>> Servlet > >>>>> bundle which internally use the existing servlets but add the > >>>>> padding. > >>>> > >>>> Time ;) > >>> > >>> Hehe. > >>> > >>> Well, for the JsonQueryServlet it is probably even simpler (and > >>> amounts > >>> to a four-some liner: > >>> > >>> if (request.getParameter("padding") != null) { > >>> response.write("padding("); > >>> } > >>> ... do the rest > >>> if (request.getParameter("padding") != null) { > >>> response.write(");"); > >>> } > >>> > >>> For the JsonRenderer, we might consider using a suffix or also a > >>> request > >>> parameter. > >>> > >>> Regards > >>> Felix > >>> > >>>> > >>>> Justin > >>>> > >>>>> > >>>>> Regards > >>>>> Felix > >>>>> > >>>>>> > >>>>>> Justin > >>>>>> > >>>>>> > >>>>>> > >>>>>>> > >>>>>>> Thank you for your help. > >>>>>>> > >>>>>>> On Fri, Apr 9, 2010 at 8:05 PM, Justin Edelson < > [email protected] > >>>>>>> <mailto:[email protected]>> wrote: > >>>>>>> > >>>>>>> These two servlets are used in different contexts, so it > >>>>>>> makes sense > >>>>>>> that you'd use different patterns for each of them. Your > >>>>>>> approach to the > >>>>>>> JsonQueryServlet looks fine to me. For the renderer servlet, > >>>>>>> I would be > >>>>>>> more inclined to create a script registered with the "jsonp" > >>>>>>> extension > >>>>>>> which is basically this: > >>>>>>> > >>>>>>> <%= request.getParameter("jsonp") %>(<% > >>>>>>> sling.include(resource.getPath(), "replaceSuffix=json") %>) > >>>>>>> > >>>>>>> Justin > >>>>>>> > >>>>>>> On 4/7/10 4:52 PM, Luca Masini wrote: > >>>>>>>> Yes, that can be great, but I don't want to change Sling > >>>>>>>> sources, only > >>>>>>>> reuse them, that's why I asked about best practices. > >>>>>>>> > >>>>>>>> On Wed, Apr 7, 2010 at 9:58 PM, Justin Edelson > >>>>>>> <[email protected] <mailto:[email protected]> > >>>>>>>> <mailto:[email protected] > >>>>>>>> <mailto:[email protected]>>> > >>>>>>> wrote: > >>>>>>>> > >>>>>>>> We should probably just support jsonp natively in these > >>>>>>>> servlets. > >>>>>>>> > >>>>>>>> > >>>>>>>> On 4/7/10 2:05 PM, Luca Masini wrote: > >>>>>>>>> Hi guys, I need an advice from Sling experts. > >>>>>>>>> > >>>>>>>>> I need a JSONP renderer for Sling, initially I thought I had to > >>>>>>>> develop only > >>>>>>>>> one, but after I discovered that in effect Sling has two > >>>>>>> renderer > >>>>>>>> for JSON, > >>>>>>>>> one for plain resources and one for query. > >>>>>>>>> > >>>>>>>>> These two renderer are in private packages in a Sling > >>>>>>> Bundle, and > >>>>>>>> because > >>>>>>>>> JSONP and JSON are quite equals and I wanted to reuse as much > >>>>>>>>> as > >>>>>>>> possible, I > >>>>>>>>> started thinking about strategies to call them. > >>>>>>>>> > >>>>>>>>> The JsonQueryServlet is deployed as an OSGi Component, so I was > >>>>>>>> able to > >>>>>>>>> inject it using Felix SCR Annotation (querying for his > >>>>>>>> properties), and then > >>>>>>>>> I called his service() method inside my doGet: > >>>>>>>>> > >>>>>>>>> servlet.service(req, resp); > >>>>>>>>> > >>>>>>>>> For the JsonRendererServlet instead I used another strategy. > >>>>>>>>> > >>>>>>>>> I extracted the RequestPathInfo from the > >>>>>>>>> SlingHttpServletRequest > >>>>>>>> and the I > >>>>>>>>> included it using RequestDispatcher: > >>>>>>>>> > >>>>>>>>> String jsonPath = calculateIt(req); > >>>>>>>>> > >>>>>>>>> req.getRequestDispatcher(jsonPath).include(req, resp); > >>>>>>>>> > >>>>>>>>> Now both of these two are working, but I have a bad taste. > >>>>>>>>> > >>>>>>>>> Which of this is the best approach ?? The first ?? The > >>>>>>>>> second ?? > >>>>>>>> None of > >>>>>>>>> them ??? > >>>>>>>>> > >>>>>>>>> > >>>>>>>> > >>>>>>>> > >>>>>>>> > >>>>>>>> > >>>>>>>> -- > >>>>>>>> **************************************** > >>>>>>>> http://www.lucamasini.net > >>>>>>>> http://twitter.com/lmasini > >>>>>>>> http://www.linkedin.com/pub/luca-masini/7/10/2b9 > >>>>>>>> **************************************** > >>>>>>> > >>>>>>> > >>>>>>> > >>>>>>> > >>>>>>> -- > >>>>>>> **************************************** > >>>>>>> http://www.lucamasini.net > >>>>>>> http://twitter.com/lmasini > >>>>>>> http://www.linkedin.com/pub/luca-masini/7/10/2b9 > >>>>>>> **************************************** > >>>>>> > >>>>>> > >>>> > >>>> > > > -- Lars Trieloff - http://lars.mp - Day Software - http://www.day.com
