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 == '\'',
On 2011/04/01 23:39:50, tbroyer wrote:
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().
I'm inclined to go with your latter suggestion and completely disallow
template variables in <meta content=...>. I can't think of any sensible
reason why someone would write a SafeHtmlTemplate with meta tags, so I
don't think this restriction would matter in practice.
Thinking about it some more; I'm wondering if a value substituted into
meta-content url context would have to be sanitized more stringently
than a "regular" uri-valued attribute. I vaguely recall there were bugs
that allowed a second URL to be injected (<meta .. content="0;
http://example.com/legit;javascript:evil()">); not sure those are still
an issue.
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>");
On 2011/04/01 23:39:50, tbroyer wrote:
Add a test for the <meta content='0;URL={0}'> case?
Done.
http://gwt-code-reviews.appspot.com/1396803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
