Skip to content

Conversation

@wackbyte
Copy link
Contributor

@wackbyte wackbyte commented Jan 14, 2024

Purpose / Goal

Changes the parsing of CDATA sections to not escape entities.

In other words:

  • Before: <[CDATA[&amp;]]> -> &
  • After: <[CDATA[&amp;]]> -> &amp;

I also added a new test so that this behavior can be tested on its own.

Please note that I did not attempt to avoid any breaking changes here as the issue suggested, but if it is desired, I would be happy to gate this fix behind a new option.

The contents of a CDATA section will also now be run through tagValueProcessor even when the cdataPropName option is provided. I am unsure as to if this inconsistency was intentional (the code that would have made it consistent seems to have been commented out a while ago), so if that is considered an unrelated change, I would also be happy to split it out into a separate PR.

Closes #632.

Performance tests

Before:

fxp v3 : 57564.501862728255 requests/second fxp : 32731.178545462913 requests/second fxp - preserve order : 35714.0221273455 requests/second xmlbuilder2 : 15299.516192662688 requests/second xml2js : 18881.74875804895 requests/second 

After:

fxp v3 : 57721.92080194106 requests/second fxp : 36231.444954502214 requests/second fxp - preserve order : 39372.370501577694 requests/second xmlbuilder2 : 15294.661359575506 requests/second xml2js : 18533.786914067532 requests/second 

Type

Please mention the type of PR

  • Bug Fix
  • Refactoring / Technology upgrade
  • New Feature
@amitguptagwl
Copy link
Member

Thanks for you PR. let me check

@coveralls
Copy link

Coverage Status

coverage: 98.251% (+0.004%) from 98.247%
when pulling 66384d1 on wackbyte:cdata
into 291fe73 on NaturalIntelligence:master.

@amitguptagwl amitguptagwl merged commit 92e07cb into NaturalIntelligence:master Jan 19, 2024
@wackbyte wackbyte deleted the cdata branch January 19, 2024 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants