Implement serializing CDATA by Ninja3047 · Pull Request #904 · tafia/quick-xml

Ninja3047 · 2025-09-25T05:27:52Z

Closes: #353

Mingun

Looks good, but need to check how lists will be serialized and would we able to deserialize them back.

src/se/simple_type.rs

codecov-commenter · 2025-09-25T13:33:25Z

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 92.14660% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.15%. Comparing base (38b44d4) to head (c526ff7).
⚠️ Report is 87 commits behind head on master.

Files with missing lines	Patch %	Lines
src/escape.rs	86.66%	4 Missing ⚠️
src/events/mod.rs	0.00%	4 Missing ⚠️
src/se/mod.rs	33.33%	4 Missing ⚠️
src/se/simple_type.rs	98.30%	2 Missing ⚠️
src/se/content.rs	87.50%	1 Missing ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #904      +/-   ##
==========================================
+ Coverage   55.52%   58.15%   +2.62%     
==========================================
  Files          42       42              
  Lines       15511    15537      +26     
==========================================
+ Hits         8613     9036     +423     
+ Misses       6898     6501     -397

Flag	Coverage Δ
unittests	`58.15% <92.14%> (+2.62%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Mingun · 2025-09-25T17:20:31Z

src/se/simple_type.rs

+        serialize_cdata_as!(seq_basic: vec!["foo", "bar", "baz"] => "<![CDATA[foo]]> <![CDATA[bar]]> <![CDATA[baz]]>");
+        serialize_cdata_as!(seq_with_markup_chars: vec!["<tag>", "&entity", "\"quoted\"", "with space"] => "<![CDATA[<tag>]]> <![CDATA[&entity]]> <![CDATA[\"quoted\"]]> <![CDATA[with space]]>");
+        serialize_cdata_as!(seq_with_cdata_end_split: vec!["foo]]>bar", "test"] => "<![CDATA[foo]]]]><![CDATA[>bar]]> <![CDATA[test]]>");
+        serialize_cdata_as!(tuple_cdata: ("first", 42, "third") => "<![CDATA[first]]> <![CDATA[42]]> <![CDATA[third]]>");


It would be better, if this serialized as one CDATA section. Probably, that is possible?

yeah agreed, need to figure out exactly how to do that nicely with serde

alright changed it so it gets serialized to one CDATA section
one thing i'm not clear on is if there are spaces inside the items, should the spaces get escaped even when it's wrapped with a CDATA

JeanMertz · 2025-10-03T08:08:56Z

Great to see this being merged soon 👍

I gave this implementation a try. One thing that confused me was that the example implies that the CDATA wrapper is only applied to fields that are marked with #[serde(rename = "$text")], but I had other String fields that also got the wrapper, even though they didn't have the $text attribute applied to them. Is this a bug, or working as expected?

JeanMertz · 2025-10-03T08:13:28Z

Also, I think the following test case should be added:

baz ]] baz

As I think(?) it generates incorrect XML:

<![CDATA[baz ]] baz ]]>

EDIT: Actually, I think it's correct, with ]]> in the middle it does split up the CDATA:

baz ]]> baz 
<![CDATA[baz ]]]]><![CDATA[> baz ]]>

Mingun · 2025-10-03T08:49:50Z

Great to see this being merged soon

I'm a bit busy last time, but I will try to find time to review this ASAP.

Is this a bug, or working as expected?

It should use CDATA as well.

EDIT: Actually, I think it's correct, with ]]> in the middle it does split up the CDATA:

Yes, having ]] which is not part of ]]> inside CDATA is totally acceptable.

JeanMertz · 2025-10-03T10:44:42Z

It should use CDATA as well.

Wouldn't it be preferable to only use CDATA if $text is defined? At least until there is an auto variant?

Mingun · 2025-10-03T12:17:53Z

I think not. Imagine, if you want to store HTML in the String field.

JeanMertz · 2025-10-03T21:44:47Z

I think not. Imagine, if you want to store HTML in the String field.

Well, then you are free to mark it as $text, but if you want to store a string that you know is safe, this PR does not give you a way to serialize that string without the CDATA tags (assuming there are other fields in the data that you do want to include the CDATA tags, which is why you'd set TextFormat to CData).

I'm not saying it's a blocker, I was mostly curious why it isn't only applied to $text fields.

…g creation

…esCData

Mingun · 2025-10-29T18:11:32Z

@Ninja3047, thanks on working on this! I finally polish the PR and make it much more simple. The release with the change will be soon

Mingun requested changes Sep 25, 2025

View reviewed changes

src/se/simple_type.rs Outdated Show resolved Hide resolved

src/se/simple_type.rs Outdated Show resolved Hide resolved

src/se/simple_type.rs Outdated Show resolved Hide resolved

src/se/simple_type.rs Outdated Show resolved Hide resolved

Mingun reviewed Sep 25, 2025

View reviewed changes

Mingun mentioned this pull request Oct 25, 2025

Remove usage of deprecated serde_if_integer128 and bump serde dependecy #908

Merged

Mingun and others added 3 commits October 28, 2025 23:47

Write escaped values into writer directly, without intermediate strin…

0fd5959

…g creation

Untie CDataIterator from the BytesCData to reuse it later without Byt…

7ab6add

…esCData

Implement serializing CDATA in SimpleTypeSerializer

4e9e8a6

Mingun force-pushed the serialize-cdata branch 2 times, most recently from 2b40fa2 to e6c93d3 Compare October 29, 2025 18:02

Implement serializing CDATA in serde's Serializer

c526ff7

Mingun force-pushed the serialize-cdata branch from e6c93d3 to c526ff7 Compare October 29, 2025 18:11

Mingun merged commit 17e575e into tafia:master Oct 29, 2025
7 checks passed

Ninja3047 deleted the serialize-cdata branch December 2, 2025 20:36

Comments

Conversation

Ninja3047 commented Sep 25, 2025

Uh oh!

Mingun left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov-commenter commented Sep 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Mingun Sep 25, 2025

Choose a reason for hiding this comment

Uh oh!

Ninja3047 Sep 25, 2025

Choose a reason for hiding this comment

Uh oh!

Ninja3047 Sep 25, 2025

Choose a reason for hiding this comment

Uh oh!

JeanMertz commented Oct 3, 2025

Uh oh!

JeanMertz commented Oct 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Mingun commented Oct 3, 2025

Uh oh!

JeanMertz commented Oct 3, 2025

Uh oh!

Mingun commented Oct 3, 2025

Uh oh!

JeanMertz commented Oct 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Mingun commented Oct 29, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov-commenter commented Sep 25, 2025 •

edited

Loading

JeanMertz commented Oct 3, 2025 •

edited

Loading

JeanMertz commented Oct 3, 2025 •

edited

Loading