Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 2 commits into
base: jdom-1.x
Choose a base branch
from

Conversation

siddhadev
Copy link

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
Copy link
Collaborator

rolfl commented Nov 25, 2013

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
Copy link
Author

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
Copy link
Collaborator

rolfl commented Nov 25, 2013

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
Copy link
Collaborator

rolfl commented Mar 30, 2014

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 added a commit that referenced this pull request Mar 30, 2014
…st case to 2.x anyway - even though code works.
rolfl added a commit that referenced this pull request Apr 5, 2014
…st case to 2.x anyway - even though code works.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants