Sorry, it took me some time but I went read the StreamHtmlParser code to better understand how it works, and how it's used here in the generator.
I thus found that there's a special-case for "meta refresh" that this patch doesn't handle (see comments below), and that the ATTR_TYPE.URI is based on HTML4 attributes only [1], while HTML5 added a few more: - formaction - icon (<command icon="">) - manifest (<html manifest="">) - poster (<video poster="">) Given that we're interested in security here, I thought I'd bring this issue. You might want to talk to the team behind JSilver (as they're all Googlers too) so they update the code, and/or patch the version of the StreamHtmlParser used in GWT, and/or handle it in the generator (instead of, or in addition to, testing isUrlStart, add a test for HTML5_URI_ATTRIBUTES.contains(streamHtmlParser.getAttribute()). LGTM otherwise. [1] http://code.google.com/p/jsilver/source/browse/trunk/src/com/google/streamhtmlparser/util/HtmlUtils.java?r=10#137 http://gwt-code-reviews.appspot.com/1396803/diff/1/user/src/com/google/gwt/safehtml/rebind/HtmlTemplateParser.java File user/src/com/google/gwt/safehtml/rebind/HtmlTemplateParser.java (right): http://gwt-code-reviews.appspot.com/1396803/diff/1/user/src/com/google/gwt/safehtml/rebind/HtmlTemplateParser.java#newcode282 user/src/com/google/gwt/safehtml/rebind/HtmlTemplateParser.java:282: Preconditions.checkState(lookBehind == '"' || lookBehind == '\'', This will fail if someone includes <meta content="0;URL={0}"> in a template, while isUrlStart will be true. This is the single case where isUrlStart() should be used instead of getValueIndex()==0 (the other special case is if insertText() was called in a ATTR_TYPE.URI, but we never call insertText()). Replacing the above isUrlStart() with getAttributeType()==ATTR_TYPE.URI && getValueIndex()==0 would make the <meta> example above fall into the HtmlContext.Type.ATTRIBUTE_VALUE case where the URL wouldn't be treated as a URL and sanitized, so it's not acceptable. The only solution seems to be to keep the isUrlStart() test but look at the char preceeding the attribute value in the template for the quote char. Something like: template.charAt(parsePosition - streamHtmlParser.getValueIndex() - 1); In most cases, getValueIndex() would be 0 so it will be equivalent to lookBehind, and in the <meta> case it would still work OK to "find" the end of the attribute value (in the <meta> case, the URL should end the attribute value, so it's OK to then compare with the 'lookAhead' char). Other solution: explicitly test for the <meta content=> case ("meta".equals(getTag()) && "content".equals(getAttribute())) and throw an UnableToCompleteException(). http://gwt-code-reviews.appspot.com/1396803/diff/1/user/test/com/google/gwt/safehtml/rebind/HtmlTemplateParserTest.java File user/test/com/google/gwt/safehtml/rebind/HtmlTemplateParserTest.java (right): http://gwt-code-reviews.appspot.com/1396803/diff/1/user/test/com/google/gwt/safehtml/rebind/HtmlTemplateParserTest.java#newcode122 user/test/com/google/gwt/safehtml/rebind/HtmlTemplateParserTest.java:122: "<a href='{0}'>{1}</a>"); Add a test for the <meta content='0;URL={0}'> case? http://gwt-code-reviews.appspot.com/1396803/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
