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

Add forceCdata option to builder to force the use of CDATA tags for all content. #396

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mdaly
Copy link

@mdaly mdaly commented Aug 8, 2017

This PR adds a forceCdata option to the XML builder that wraps all text nodes in CDATA even when it is not required. This option is set to false by default. A new test exercises the forceCdata option using a small sample of text that does not require escaping.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 97.633% when pulling 16ce472 on mdaly:force-cdata-builder-option into 434abf4 on Leonidas-from-XIV:master.

@Leonidas-from-XIV
Copy link
Owner

Why would I want to wrap things in unnecessary CDATA?

@mdaly
Copy link
Author

mdaly commented Aug 8, 2017

We've run into a situation where a parser (that we don't control) requires CDATA on all text content, like what's described in #321. I'm not sure how common this is, but the ability to always force CDATA has served us well.

@Leonidas-from-XIV
Copy link
Owner

Truth be told, I am not very fond of outputting nonsensical XML just because something that claims it parses XML does not do it properly. It is like saying "we have a parser that doesn't handle characters, so we should encode every character via XML entities".

@superl2080
Copy link

@Leonidas-from-XIV We met the same problem, some company require CDATA on all text content, but we can't say they're wrong and fix for us. So we have to change our code in workspace use CDATA force. We hope there'll be coming a long-term solution.

@sirius1024
Copy link

same problem here

@kirpichenko
Copy link

Same for us. One of the german services requires all the data to be wrapped into CDATA

@noumantahir
Copy link

In my case, I have a bunch of attributes containing special characters like "&", it gets converted to to "$amp;"...is there any way I can avoid this add make all my attributes retain values using CDATA?

@ryanthegiantlion
Copy link

Same problem here

@kennylbj
Copy link

+1
The Tecent wechat api require CDATA on all content.

@std4453
Copy link

std4453 commented May 17, 2019

Same problem with Wechat. Now I have to use a template string instead of xml2js to force CDATA. Painful as it might, I still consider a forceCdata option quite useful.

@tal-rofe
Copy link

tal-rofe commented Jul 2, 2022

Anything?

@dmitryshk
Copy link

Same problem with Wechat. Now I have to use a template string instead of xml2js to force CDATA. Painful as it might, I still consider a forceCdata option quite useful.

Just wanted to elaborate on this suggestion. It could be implemented as the following:

  1. Walk through all object fields that need to be in CDATA tag and append to their values a string that will force CDATA generation e.g. value += '<FORCECDATA>'.
  2. Build XML.
  3. Remove all occurrences of the appended string from the XML string, e.g. xml = xml.ReplaceAll('<FORCECDATA>', '').

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet