Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

merging CDATAs can producing an ending tag ]]> in the resulting CDATA leading to an exception #124

Open
wants to merge 2 commits into from

2 participants

@siddhadev

Imagine you have the following XML input:

     <message priority="info"><![CDATA[  expected:<[[D/0]]]]><![CDATA[> but was:<[null]>]]></message>

this would be a legal XML representing the following string:

expected:<[[D/0]]> but was: <[null]>

now when SAXHandler.characters() gets called, it would try to merge subsequent CDATAs, and call setText() which would fail with:

Caused by: org.jdom.IllegalDataException: The data "  expected:<[[D/0]]> but was:<[null]>" is not legal for a JDOM CDATA section: CDATA cannot internally contain a CDATA ending delimiter (]]>).
at org.jdom.CDATA.setText(CDATA.java:121)
at org.jdom.CDATA.<init>(CDATA.java:95)
at org.jdom.DefaultJDOMFactory.cdata(DefaultJDOMFactory.java:97)
at org.jdom.input.SAXHandler.flushCharacters(SAXHandler.java:652)
at org.jdom.input.SAXHandler.flushCharacters(SAXHandler.java:623)
at org.jdom.input.SAXHandler.endElement(SAXHandler.java:678)
at org.apache.xerces.parsers.AbstractSAXParser.endElement(Unknown Source)

It's not a nice fix, but its an easy workaround.

@rolfl
Collaborator

Good catch. And appreciate the patch... but I am not particularly happy with the fix.... I think there's an issue in it:

if (previousCDATA != inCDATA || (ch[start] == '>' || (ch[start] == ']' && ch[start] == '>') ))

(ch[start] == ']' && ch[start] == '>') cannot possibly be true....

The situation is that the 'real' data contains a closing CDATA tag ']]>' and this is 'split' between two 'real' CDATA sections... so, the fix really needs to span what's been seen already, and what's just arrived...

I realize that the bug is real, but this fix is incomplete... If you want to have another stab at it, feel free, and I'll pull it in, but as it stands it's not ready. Otherwise, I'll take a look at it myself in a day or two.

@siddhadev

Oh, you are right, it was meant like this:

ch[start] == ']' && ch[start+1] == '>'

I updated the pull request, but you don't have to use it, a better fix should be possible. I guess a CDATA could get broken by more than just a closing tag, i.e. "]]>".

@rolfl
Collaborator

I am going to contemplate this issue for a bit. I think I may have an alternative approach which is more reliable. I will have to take the time to play with the code though.

@rolfl
Collaborator

Just so you are aware, this issue is not a problem in JDOM 2.x , and I strongly recommend you upgrade to that. It is unlikely that there will be another JDOM 1.x release any time soon.

@rolfl rolfl added the Target 1.1.x label
@rolfl rolfl referenced this pull request from a commit
@rolfl rolfl Issue #124 - Split CDATA sections causing trouble in JDOM 1.x. Add te…
…st case to 2.x anyway - even though code works.
b3b7ae3
@rolfl rolfl referenced this pull request from a commit
@rolfl rolfl Issue #124 - Split CDATA sections causing trouble in JDOM 1.x. Add te…
…st case to 2.x anyway - even though code works.
5c2080c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 1 addition and 1 deletion.
  1. +1 −1  core/src/java/org/jdom/input/SAXHandler.java
View
2  core/src/java/org/jdom/input/SAXHandler.java
@@ -731,7 +731,7 @@ public void characters(char[] ch, int start, int length)
if (suppress || (length == 0))
return;
- if (previousCDATA != inCDATA) {
+ if (previousCDATA != inCDATA || (ch[start] == '>' || (ch[start] == ']' && length > 1 && ch[start+1] == '>') )) {
flushCharacters();
}
Something went wrong with that request. Please try again.