Sure, I will add a note about the intended usage in the code. I will be moving the existing widgets over to SafeHtml soon, and that will, hopefully, provide some good examples as well.
Philip On Thu, Aug 19, 2010 at 10:42 AM, Arthur Kalmenson <[email protected]>wrote: > I'm just wondering, the idea of this is to display potentially unsafe > HTML we got from the server, right? Is there a way to discourage using > this to sanitizing HTML on the client before sending it to the server? > I can definitely see newer programmers assuming that doing this > sanitization on the client side is safe enough, when it's clearly not. > I guess some documentation to that effect would be enough. > > -- > Arthur Kalmenson > > > > On Wed, Aug 18, 2010 at 10:11 AM, Christoph Kern <[email protected]> wrote: > > > > > > On Wed, Aug 18, 2010 at 01:44, <[email protected]> wrote: > >> > >> On 2010/08/17 23:23:39, xtof wrote: > >>> > >>> On 2010/08/17 23:05:06, tbroyer wrote: > >>> > > >>> > Looking at the code more closely it would merely "fail" by overly > >>> > rejecting tags that are whitelisted: i.e. "<b foo=<i>should be > >>> > bold" would be sanitized to "<b foo=<i>should be bold" and the > >>> > end part would be italicized instead of bold. > >> > >>> And that is exactly correct behavior for this class as document. It > >>> only claims to accept HTML with attribute-free tags within the > >>> whitelist. It doesn't claim to do anything particularly sensible > >>> with input that doesn't fit this constraint; it does however claim > >>> that whatever it outputs is safe (will not result in XSS/script > >>> execution). > >> > >> Oops, yes, sorry, I can't tell how it happened but I misread the > >> "whitelisting" code (matches the whole thing between '<' and '>', so any > >> attribute, or even whitespace, as in "<b >bold</b>", would make it fail > >> and thus be escaped). > >> It's fine then. Sorry again for the noise. > > > > No prob, always better to err on the side of caution! > > > >> > >> Still, there's a small issue with the fact that > >> SafeHtmlTemplatesGenerator doesn't use the HTML5 serialization algorithm > >> (or any similar one): @Template("<br/>") will result in "<br></br>" > >> which is interpreted by browsers as "<br><br>" [1], which makes it > >> impossible to generate a single "line break" in a SafeHtmlTemplates. > >> > >> [1] http://www.w3.org/TR/html5/tokenization.html#parsing-main-inbody > >> (search for « An end tag whose tag name is "br" », it's there for > >> "compat with the Web") > > > > Agreed. Another potential problem with this parser is that it's too > strict > > -- it insists on well-formed XHTML, but that's a much stronger constraint > > than needed to ensure the SafeHtml type contract. > > For example, > > @Template("<a href=\"{0}\">") > > SafeHtml openATag(String href); > > is perfectly safe, and might be useful in something where one needs to > > conditionally assemble various pieces of HTML markup, like, > > SafeHtmlBuilder shb; > > //... > > shb.append(openATag(someUrl)); > > if (...) { > > shb.append(someThing) > > } else { > > shb.append(someThingElse); > > } > > To ensure the SafeHtml type contract, the parser need only record the > > HTML-context ("inner text", "url-valued attribute", "style", etc) of the > > positional parameters, and require that the template ends in "inner text" > > context (to ensure that SafeHtmls are safely appendable to each other). > > Note that a bug in code like the above could result in non-well-formed > HTML > > (like unbalanced tags), but not unsafe HTML that might cause XSS (that is > of > > course absent bugs in the SafeHtml generators themselves). > > As I'd mentioned, I'm proposing to replace the current XML based parser > with > > the java streamhtmlparser, which has this property. > > I'd be fine with removing the templating code from this change and land > it > > separately along with the new parser. > > cheers, > > --xtof > > > > > >> > >> > >> http://gwt-code-reviews.appspot.com/771801/show > > > > -- > > http://groups.google.com/group/Google-Web-Toolkit-Contributors > > -- > http://groups.google.com/group/Google-Web-Toolkit-Contributors > -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
