Prevent double CDATA section escaping in filter_dom_serialize_escape_cdata_element() to avoid warnings

Could you provide some more information for us? Like a click-through recipe we could follow of steps we can do to see this happen?

I met the same report.
They are:

Error message
Warning: DOMDocumentFragment::appendXML(): Entity: line 15: parser error : StartTag: invalid element name in filter_dom_serialize_escape_cdata_element() (line 1108 of /home/charlesd/public_html/d7_kno/modules/filter/filter.module).
............

It'sl weird that, I run a drupal 7 at 127.0.0.1(own pc), it's ok,
but run drupal at byethost13.com(free host) then I have this problem(above),
so I check "Reports -> Status Reports" and I found 2 problems(below), how to solve them, please?

1.

Notice: Undefined variable: errno in drupal_http_request() (line 829 of /home/vol4/byethost32.com/b32_7294076/hellorains.iblogger.org/htdocs/index/includes/common.inc).
Notice: Undefined variable: errstr in drupal_http_request() (line 830 of /home/vol4/byethost32.com/b32_7294076/hellorains.iblogger.org/htdocs/index/includes/common.inc).

and

2.

HTTP request status	Fails
Your system or network configuration does not allow Drupal to access web pages, resulting in reduced functionality. This could be due to your webserver configuration or PHP settings, and should be resolved in order to download information about available updates, fetch aggregator feeds, sign in via OpenID, or use other network-dependent services. If you are certain that Drupal can access web pages but you are still seeing this message, you may add $conf['drupal_http_request_fails'] = FALSE; to the bottom of your settings.php file.

sorry I forget one thing:
I post a content then I met this problem:

<script src="rainsjs/jquery.js"></script>

How to reproduce:
1. Set the "Full HTML" text format (admin/config/content/formats) to "Correct faulty and chopped off HTML"
2. Create/Edit a Basic Page node
3. Put the following code in the body using the "Full HTML" text format and save:

<script type="text/javascript">// <![CDATA[
alert('hi');
// ]]></script>

END OF REPRODUCTION STEPS

I've had this problem with TinyMCE. First you have to set TinyMCE to allow script tags using hook_wysiwyg_plugin(). Then when editing a node, when you insert some javascript as so:

<script type="text/javascript">alert('hi');</script>

and save, everything is fine. When Drupal renders the node, the filter module adds a CDATA wrapper, and the JS executes o.k. and everything is still fine. However, when you edit that node for the first time, TinyMCE adds its own CDATA wrapper like so:

<script type="text/javascript">// <![CDATA[
alert('hi');
// ]]></script>

When you save that you get the errors OP showed and Drupal renders:

<script type="text/javascript">
</script>

I had this problem after adding a couple of blog posts, what I found looking under source was in one post was extra css tags that the system had some how generated. I deleted the post and re-entered it and that fixed it.

In my case this chunk of HTML

<!--//--><![CDATA[// ><!--

      if (Drupal.settings && Drupal.media_youtube) {
        Drupal.settings.media_youtube = Drupal.settings.media_youtube || {};
        Drupal.settings.media_youtube["media_youtube_Hyi4hjG6kDM_1"] = {};
        Drupal.settings.media_youtube["media_youtube_Hyi4hjG6kDM_1"].width = 640;
        Drupal.settings.media_youtube["media_youtube_Hyi4hjG6kDM_1"].height = 390;
        Drupal.settings.media_youtube["media_youtube_Hyi4hjG6kDM_1"].video_id = "Hyi4hjG6kDM";
        Drupal.settings.media_youtube["media_youtube_Hyi4hjG6kDM_1"].fullscreen = true;
        Drupal.settings.media_youtube["media_youtube_Hyi4hjG6kDM_1"].id = "media_youtube_Hyi4hjG6kDM_1_iframe";
        
        Drupal.media_youtube.insertEmbed("media_youtube_Hyi4hjG6kDM_1");
      }
    
//--><!]]>

Throws the warnings and gets turned into this:

<!--//--><![CDATA[// ><!--

<!--//--><![CDATA[// ><!--

      if (Drupal.settings && Drupal.media_youtube) {
        Drupal.settings.media_youtube = Drupal.settings.media_youtube || {};
        Drupal.settings.media_youtube["media_youtube_Hyi4hjG6kDM_1"] = {};
        Drupal.settings.media_youtube["media_youtube_Hyi4hjG6kDM_1"].width = 640;
        Drupal.settings.media_youtube["media_youtube_Hyi4hjG6kDM_1"].height = 390;
        Drupal.settings.media_youtube["media_youtube_Hyi4hjG6kDM_1"].video_id = "Hyi4hjG6kDM";
        Drupal.settings.media_youtube["media_youtube_Hyi4hjG6kDM_1"].fullscreen = true;
        Drupal.settings.media_youtube["media_youtube_Hyi4hjG6kDM_1"].id = "media_youtube_Hyi4hjG6kDM_1_iframe";
        
        Drupal.media_youtube.insertEmbed("media_youtube_Hyi4hjG6kDM_1");
      }
    
//--><!]]>

//--><!]]>

I am getting the same error when I attempt to add blocks with html code. The v7 installation is new, nothing has been migrated from earlier versions.

I have set the "Full HTML" format to "Correct faulty and chopped off HTML".

I have created new code for the box, nothing helps.

Any clues how to fix this?

thx! this help me and show, to have more attention on what i am doin when creating von of my own-contenttyped-page by copy and paste from the website which i will copy into a drupal version, realize it or build it with drupal.

i had a look to the source of the page in my ckeditor and found a javascript where no script has to be.
delete - and the messages wont come again.

thank you all :)

Hello.

I am receiving that error message just by having <style> tags whether in the Summary or Body areas.

I'm using version 7.4.

EDIT: I was using the plain text editor at the time. The error went away when I switched to CKEditor.

Subs, I may need to look at this soon.

We are getting these same symptoms on twitter feed .js files. It seems it maybe a conflicting module, as we have enabled a host of them on our dev site... more later:(

More on this issue, seems to be the following:

Under Configuration >> Content Authoring >> Full HTML >> [tick box] Correct faulty and chopped off HTML fixes this issue as stated in the explanation on how to reproduce the issue with

Sorry for any coders but for non-coders this last step is nasty. So make sure and remove the {open tag} and the {close tag}

DrupalUP Nasty 4sure.

More on this issue, seems to be the following:

Under Configuration >> Content Authoring >> Full HTML >> [tick box] Correct faulty and chopped off HTML fixes this issue as stated in the explanation on how to reproduce the issue with [CDATA[ appearing in the ckeditor at any point of edit. Then by actually saving the file if you do not manually remove the code from the actual editor you get the stated error thrown above.

So there are a few ways around this, but as of yet there is not an actual FIX that we have found.

Note: If you want to remove the CDATA make sure and get it all or you will still get this error.
Example

Sorry for any coders but for non-coders this last step is nasty. So make sure and remove the {open tag} and the {close tag}

DrupalUP Nasty 4sure.

More on this issue, seems to be the following:

Under Configuration >> Content Authoring >> Full HTML >> [tick box] Correct faulty and chopped off HTML fixes this issue as stated in the explanation on how to reproduce the issue with [CDATA[ appearing in the ckeditor at any point of edit. Then by actually saving the file if you do not manually remove the code from the actual editor you get the stated error thrown above.

So there are a few ways around this, but as of yet there is not an actual FIX that we have found.

Note: If you want to remove the CDATA make sure and get it all or you will still get this error.
Example

Sorry for any coders but for non-coders this last step is nasty. So make sure and remove the {open tag} and the {close tag}

DrupalUP Nasty 4sure.

Can somebody tell me how to fix this error? I don't have any CDATA coding, and I went through these forums. Here is my homepage: http://jrhh.biz

This seemed to happen when I put the google analytics code in at the top of my front page (hompage), but I'm not sure.

I posted a pic of what it looks like.

Here's the full message:

Warning: DOMDocumentFragment::appendXML() [domdocumentfragment.appendxml]: Entity: line 20: parser error : StartTag: invalid element name in filter_dom_serialize_escape_cdata_element() (line 1107 of /home4/searchs1/public_html/JrHH.biz/modules/filter/filter.module).
Warning: DOMDocumentFragment::appendXML() [domdocumentfragment.appendxml]: //--> in filter_dom_serialize_escape_cdata_element() (line 1107 of /home4/searchs1/public_html/JrHH.biz/modules/filter/filter.module).
Warning: DOMDocumentFragment::appendXML() [domdocumentfragment.appendxml]: ^ in filter_dom_serialize_escape_cdata_element() (line 1107 of /home4/searchs1/public_html/JrHH.biz/modules/filter/filter.module).
Warning: DOMDocumentFragment::appendXML() [domdocumentfragment.appendxml]: Entity: line 20: parser error : Sequence ']]>' not allowed in content in filter_dom_serialize_escape_cdata_element() (line 1107 of /home4/searchs1/public_html/JrHH.biz/modules/filter/filter.module).
Warning: DOMDocumentFragment::appendXML() [domdocumentfragment.appendxml]: //--> in filter_dom_serialize_escape_cdata_element() (line 1107 of /home4/searchs1/public_html/JrHH.biz/modules/filter/filter.module).
Warning: DOMDocumentFragment::appendXML() [domdocumentfragment.appendxml]: ^ in filter_dom_serialize_escape_cdata_element() (line 1107 of /home4/searchs1/public_html/JrHH.biz/modules/filter/filter.module).
Warning: DOMDocumentFragment::appendXML() [domdocumentfragment.appendxml]: Entity: line 20: parser error : Sequence ']]>' not allowed in content in filter_dom_serialize_escape_cdata_element() (line 1107 of /home4/searchs1/public_html/JrHH.biz/modules/filter/filter.module).
Warning: DOMDocumentFragment::appendXML() [domdocumentfragment.appendxml]: //--> in filter_dom_serialize_escape_cdata_element() (line 1107 of /home4/searchs1/public_html/JrHH.biz/modules/filter/filter.module).
Warning: DOMDocumentFragment::appendXML() [domdocumentfragment.appendxml]: ^ in filter_dom_serialize_escape_cdata_element() (line 1107 of /home4/searchs1/public_html/JrHH.biz/modules/filter/filter.module).
Warning: DOMDocumentFragment::appendXML() [domdocumentfragment.appendxml]: Entity: line 20: parser error : internal error in filter_dom_serialize_escape_cdata_element() (line 1107 of /home4/searchs1/public_html/JrHH.biz/modules/filter/filter.module).
Warning: DOMDocumentFragment::appendXML() [domdocumentfragment.appendxml]: //--> in filter_dom_serialize_escape_cdata_element() (line 1107 of /home4/searchs1/public_html/JrHH.biz/modules/filter/filter.module).
Warning: DOMDocumentFragment::appendXML() [domdocumentfragment.appendxml]: ^ in filter_dom_serialize_escape_cdata_element() (line 1107 of /home4/searchs1/public_html/JrHH.biz/modules/filter/filter.module).
Warning: DOMNode::appendChild() [domnode.appendchild]: Document Fragment is empty in filter_dom_serialize_escape_cdata_element() (line 1108 of /home4/searchs1/public_html/JrHH.biz/modules/filter/filter.module).

Problem/Motivation

Under some circumstances warnings are thrown and code is stripped if inline javascript / css is inserted in Full HTML input fields.
Notice:

Warning: DOMDocumentFragment::appendXML() [domdocumentfragment.appendxml]: Entity: line 20: parser error : StartTag: invalid element name in filter_dom_serialize_escape_cdata_element() (line 1107 of /modules/filter/filter.module).

The mentioned function filter_dom_serialize_escape_cdata_element() intends to properly escape CDATA sections, by adding code like <!--//--><![CDATA[// ><!--, on DOMNodes with the type DOMCdataSection.
Currently the function doesn't prevent double escaping and unfortunately DOMDocumentFragment::appendXML() throws a warning if such an escaping is doubled.

Proposed resolution

The only way I currently can imagine to get this fixed is to manually search for an already existing CDATA section escaping and if so skip the additional escaping.

The attached patch adds a check to determine if / which CDATA section escaping part has to be added.
The existing tests are extended to cover several different scenarios.

Original report by Charles51

Error message
Warning: DOMDocumentFragment::appendXML(): Entity: line 15: parser error : StartTag: invalid element name in filter_dom_serialize_escape_cdata_element() (line 1108 of /home/charlesd/public_html/d7_kno/modules/filter/filter.module).
Warning: DOMDocumentFragment::appendXML(): //--> in filter_dom_serialize_escape_cdata_element() (line 1108 of /home/charlesd/public_html/d7_kno/modules/filter/filter.module).
Warning: DOMDocumentFragment::appendXML(): ^ in filter_dom_serialize_escape_cdata_element() (line 1108 of /home/charlesd/public_html/d7_kno/modules/filter/filter.module).
...

Created initial summary.

Added nicer title and give the testbot a shot.

As recommended by xjm via IRC I separated the patch into actual fix and new tests. If you apply the tests first you can see what fails. And after applying the fix, all tests will pass.

Updated issue summary. Added link to current patch

All bugs are fixed in latest core version first.

The patch containing the fix should also contain the tests as in the tests-only patch.

Lastly, while the fix looks reasonable, it kinda contradicts the entire point of DOM loading/serialization - which is to NOT use PCREs, since PCREs don't work on HTML.

• Luckily I've found some time and delight to reroll against 8.x .
• Sorry for that - was a misunderstanding of xjm's advice. Attached now 1) full patch 2) tests only.
• Indeed, ideas for a better approach would be very welcome.
  But while I know I'll go straight to hell for parsing HTML/XML/XHTML with regular expressions I simply don't know a better way (but I'm willing to learn).
  As far as I know the DOM parser can't / shall not parse CDATA as by the basic definition of a CDATA section.

After a good talk with Pisco here we go with another, regexp-less, approach.
The patch now uses the same approach as Libxml2.
Related links:

• Related W3C standard: http://www.w3.org/TR/REC-xml/#dt-cdsection
• Fix explanation: http://en.wikipedia.org/wiki/CDATA#Nesting

sun demanded more specific tests - I've changed the tests to use explicit string comparison.

Let's go with #26 so that it can be commited to D7 . The real fix is to elevate #1277290: Use a proper HTML parser for every core filter to major task and do it.

Edit: to clarify, I can break #26 in a heartbeat with <script>alert(']]>');</script> however it is vastly better than what we have now and short of fixing the PHP DOM extension to employ the hack in https://bugzilla.gnome.org/show_bug.cgi?id=357992 it is not possible to fix this fully using the DOM HTML parser.

So the issue has a frightening title but this is a simple patch in the end, and comes with lots of lovely looking tests.

Committed/pushed to 8.x, we should bump that other issue to major if there's a way to avoid adding kludges like that, but completely refactoring this in Drupal 7 doesn't seem like an option.

*yay* thanks catch.
Attached is the reroll agains the latest D7-dev

Works for me. :)

#30 fixed it for me too! Thx!
(even though I wonder why these errors suddenly started popping up today, I didn't change anything compared to yesterday.)

#30 confirmed ++

Why doesn't anybody update the ticket status???

Why doesn't anybody update the ticket status???

Because reporting that the patch has the desired result is different from vouching for the patch as the best way to fix the problem. If you are prepared to take that responsibility, go for it.

I really don't know what you are trying to say. Do we need patches that solve problems in a best way? Is there a best way at all? And if there actually would be a best way, how would I know I've found it? How could I claim that no one could ever come up with an even better solution? Because If I already had found the best way, then there would be absolutely no way for someone to ever come up with a better one … because I would already have the best way, right?

No, I don't think we absolutely need best ways to solve problems, good ways are just fine. And even better if we can have them now instead of sometimes in an undefined future. A bird in the hand might be worth two in the bush.

Drupal uses a relatively rigid peer-review process to ensure that we find and implement the best possible solution for every issue that arises. "Best" is obviously relative to the current point in time, the environment, and also the involved contributors; but within that constellation, "best" actually does mean the "best". It does not necessarily mean "ideal," since that might be outside of the constraints of the relative "best possible" solution. But in fact, as also visible in this very issue, we certainly want to identify the root cause and ideal solution in order to implement it as a follow-up in the new version of Drupal, which is generally unconstrained and thus, involves different relative design parameters than the current best solution.

Yes, this patch is the backport of the best way we were able to come up with to fix the bug - under the constraints of the existing API.

Thanks for the info sun. I helped das-peter design this patch and I think it is a good and solid solution for the bug that was found. It solves it by the same approach as libxml, which I think can be viewed as reference implementation, and that makes me confident. What is good for libxml is good enough for Drupal.
Independently of what you might say about yourself, I would never claim of a solution I helped design, that it is in any way the best one could come up with, but that's just how I see things. Having helped design the patch, I'm not the one to review it, because that wouldn't be in the spirit of our review process, therefore I was reluctant to update the ticket status.

But let's look at was has happend: the patch has been improved and adjudged apt for inclusion into D8 in #29. das-peter, the author of the original patch, then backported the it for D7 in #30. 4 independent users have confirmed that the patch actually solves the problem from their point of view. Again I ask: why doesn't anybody update the ticket status? What's the point in 4 people telling that the patch works but not updating the ticket status?

I someone who tested patch has reservations, it would be helpful to hear the objections. Only then can one try to improve the situation. Just saying "Thanks for the patch, it solved my problem" and not caring to help the process move forward doesn't help Drupal as a whole. Let's work together and push things forward.

Do you object me updating the ticket status? If so, why? How do you think should be patch be improved? Has it become superfluous for D7 in the meantime? If so, show your disagreement by updating the ticket status.

If not, then please let us not bother with my comment in #36. Instead let us be happy that a problem has been potentially fixed, hopefully without to much, yet unnoticed, collateral damage, and move forward improving Drupal.

It's works for me repeatedly!
Thanks!

How do I apply the patch?

@Pio: http://drupal.org/patch/apply

Patch work great for me, please get this in.

I came from #1340018: Multiple errors after updating modules. - thanks @chx

I got this error:
Warning: DOMNode::appendChild() [domnode.appendchild]: Document Fragment is empty in filter_dom_serialize_escape_cdata_element() (line 1112 of /DRUPAL7/modules/filter/filter.module).

Applied the patch to a migration, rolled the migration back an then re-imported it and there were no errors. I'd say this is patch does what is needed.

Hoping it gets into the next release.

Also worked for me. Thank you!

As for applying the patch, searching Drupal.org for "how to apply a patch" quickly found the answer (e.g. Applying patches on Mac OS X). I've done it many times before but so infrequently that I have to re-learn how to do it each time... There are so many scenarios to consider for this - I think it would be difficult and inappropriate to attempt to answer the question in a issue/comment queue.

Patch in #30 worked well for me as well. Thanks!

Cool, thanks for the testing and the automated tests!

Committed and pushed to 7.x. Thanks!

Thank you!

thanks! the patch at #30 fixed it for me as well!

The patch at #30 worked!

Fixing tags per http://drupal.org/node/1517250

Removing obsolete tag.

Updated issue summary. Updated link to latest patch