[Caja] Fixing IllegalStateException when serializing

This patch fixes Nodes.render() to not throw IllegalStateException when CDATA section contains a close tag in MarkupRenderMode.HTML4_BACKWARDS_COMPAT mode.

Exception stack trace:
java.lang.IllegalStateException: XML document not renderable as HTML due to </script in CDATA tag
       at com.google.caja.parser.html.Renderer.render(Nodes.java:370)
       at com.google.caja.parser.html.Renderer.render(Nodes.java:379)
       at com.google.caja.parser.html.Renderer.render(Nodes.java:379)
       at com.google.caja.parser.html.Renderer.render(Nodes.java:274)
       at com.google.caja.parser.html.Nodes.render(Nodes.java:219)
       at com.google.caja.parser.html.Nodes.render(Nodes.java:233)
       at com.google.caja.parser.html.Nodes.render(Nodes.java:244)
       at org.apache.shindig.gadgets.parse.caja.BaseCajaHtmlSerializer.serialize(BaseCajaHtmlSerializer.java:25)
       at org.apache.shindig.gadgets.parse.HtmlSerialization.serialize(HtmlSerialization.java:106)
       at org.apache.shindig.gadgets.rewrite.MutableContent.getRawContentBytes(MutableContent.java:160)
       at org.apache.shindig.gadgets.http.HttpResponseBuilder.getResponse(HttpResponseBuilder.java:271)
       at org.apache.shindig.gadgets.http.HttpResponse.<init>(HttpResponse.java:168)
       at org.apache.shindig.gadgets.http.HttpResponseBuilder.create(HttpResponseBuilder.java:87)
       at org.apache.shindig.gadgets.rewrite.DefaultResponseRewriterRegistry.rewriteHttpResponse(DefaultResponseRewriterRegistry.java:59)
       at org.apache.shindig.gadgets.servlet.AccelHandler.doFetch(AccelHandler.java:92)
       at org.apache.shindig.gadgets.servlet.ProxyBase.fetch(ProxyBase.java:165)
       at org.apache.shindig.gadgets.servlet.HtmlAccelServlet.doGet(HtmlAccelServlet.java:63)

[Kuntal Loya, 2010-12-20 08:38:54]

http://codereview.appspot.com/3280043/diff/6001/tests/com/google/caja/parser/...
File tests/com/google/caja/parser/html/DomParserTest.java (right):

http://codereview.appspot.com/3280043/diff/6001/tests/com/google/caja/parser/...
tests/com/google/caja/parser/html/DomParserTest.java:286: + "<script
src=\"ur1\"></scriptscript src='url2'></scrip</body></html>"
Does this mean, have two closing body and html tags?
That doesn't make sense - Caja should remove the dups.

I am not sure how the browser would treat this funny </scrip tag.

[gagan.goku, 2010-12-20 09:28:06]

http://codereview.appspot.com/3280043/diff/6001/tests/com/google/caja/parser/...
File tests/com/google/caja/parser/html/DomParserTest.java (right):

http://codereview.appspot.com/3280043/diff/6001/tests/com/google/caja/parser/...
tests/com/google/caja/parser/html/DomParserTest.java:286: + "<script
src=\"ur1\"></scriptscript src='url2'></scrip</body></html>"
On 2010/12/20 08:38:54, Kuntal Loya wrote:
> Does this mean, have two closing body and html tags?
> That doesn't make sense - Caja should remove the dups.
> 
> I am not sure how the browser would treat this funny </scrip tag.

It is not a duplicate. I tried the same html on both webkit and firefox, and in
both cases, the text content of the script tag is:
</scriptscript src='url2'></scrip</body></html>

You are seeing </body> in serialized html twice because the first one is in text
content of script tag, and 2nd one is the ending </body> inserted by the parser.

The same thing happens when you print the documents innerHTML in the firebug
console.
Tried document.documentElement.innerHTML on webkit, and it gives same html as
our serialized one.

Take a look at the updated test case.

[Kuntal Loya, 2010-12-21 06:18:09]

lgtm

[Jasvir, 2010-12-21 07:46:48]

http://codereview.appspot.com/3280043/diff/11001/src/com/google/caja/parser/h...
File src/com/google/caja/parser/html/Nodes.java (right):

http://codereview.appspot.com/3280043/diff/11001/src/com/google/caja/parser/h...
src/com/google/caja/parser/html/Nodes.java:373: mode !=
MarkupRenderMode.HTML4_BACKWARDS_COMPAT) {
I cannot find the reference right now but I am pretty sure html4 spec also says
that a "</" ends a CDATA section.  For example:
<script>
alert("</script><script>top.location='http://www.thinkfu.com';//");
</script>

...appears to be a script that contains a single (harmless) CDATA section but
really is two scripts, the latter redirects.

http://codereview.appspot.com/3280043/diff/11001/tests/com/google/caja/parser...
File tests/com/google/caja/parser/html/DomParserTest.java (right):

http://codereview.appspot.com/3280043/diff/11001/tests/com/google/caja/parser...
tests/com/google/caja/parser/html/DomParserTest.java:280: assertEquals(TEST_NAME
+ ":4+66: End of file seen inside an "
Please use assertMessage instead.

[Jasvir, 2010-12-21 07:47:58]

Found it.  http://www.w3.org/TR/html4/types.html#h-6.2

[gagan.goku, 2010-12-21 19:50:29]

http://codereview.appspot.com/3280043/diff/11001/src/com/google/caja/parser/h...
File src/com/google/caja/parser/html/Nodes.java (right):

http://codereview.appspot.com/3280043/diff/11001/src/com/google/caja/parser/h...
src/com/google/caja/parser/html/Nodes.java:373: mode !=
MarkupRenderMode.HTML4_BACKWARDS_COMPAT) {
On 2010/12/21 07:46:48, jasvir wrote:
> I cannot find the reference right now but I am pretty sure html4 spec also
says
> that a "</" ends a CDATA section.  For example:
> <script>
> alert("</script><script>top.location='http://www.thinkfu.com';//");
> </script>
> 
> ...appears to be a script that contains a single (harmless) CDATA section but
> really is two scripts, the latter redirects.

As discussed, replaced all occurances of "</" with "< /" in html mode.

http://codereview.appspot.com/3280043/diff/11001/tests/com/google/caja/parser...
File tests/com/google/caja/parser/html/DomParserTest.java (right):

http://codereview.appspot.com/3280043/diff/11001/tests/com/google/caja/parser...
tests/com/google/caja/parser/html/DomParserTest.java:280: assertEquals(TEST_NAME
+ ":4+66: End of file seen inside an "
On 2010/12/21 07:46:48, jasvir wrote:
> Please use assertMessage instead.

Done.

[racarpad45, 2010-12-28 15:06:38]

video