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

[Serializer] Add XmlEncoder::CDATA_WRAPPING_PATTERN context option #54663

Conversation

alexpozzi
Copy link
Contributor

@alexpozzi alexpozzi commented Apr 18, 2024

Q A
Branch? 7.1
Bug fix? no
New feature? yes
Deprecations? no
Issues Fix #54155
License MIT

First of all thank you for all your hard work!

This PR adds the ability to configure the CDATA wrapping pattern to give more flexibility on when to wrap values in a CDATA section.
For example, XML validators are not allowing double and single quotes outside of a CDATA section, with this change we could be able to change the pattern from /[<>&]/ to /[<>&"\']/ and solve that issue without the need of writing a custom XMLEncoder.

@alexpozzi alexpozzi requested a review from dunglas as a code owner April 18, 2024 14:47
@carsonbot carsonbot added this to the 7.1 milestone Apr 18, 2024
@alexpozzi alexpozzi force-pushed the issue-54155/add-the-ability-to-confiure-cdata-wrapper-pattern branch 4 times, most recently from 39f0428 to 105a952 Compare April 18, 2024 14:55
@OskarStark OskarStark changed the title [Serializer] Add XmlEncoder::CDATA_WRAPPING_PATTERN context option [Serializer] Add XmlEncoder::CDATA_WRAPPING_PATTERN context option Apr 19, 2024
Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix @OskarStark's comments and GTM, thanks.

@fabpot fabpot modified the milestones: 7.1, 7.2 Apr 27, 2024
@alexpozzi alexpozzi force-pushed the issue-54155/add-the-ability-to-confiure-cdata-wrapper-pattern branch 2 times, most recently from 5e25e13 to 06ccf50 Compare April 29, 2024 06:29
@alexpozzi
Copy link
Contributor Author

Hello @nicolas-grekas,
I'm not so sure to understand what you mean by GTM.
I fixed the comments and rebased to the latest 7.1 commit.

@alexpozzi alexpozzi force-pushed the issue-54155/add-the-ability-to-confiure-cdata-wrapper-pattern branch from 06ccf50 to 43e0750 Compare April 29, 2024 06:33
@OskarStark
Copy link
Contributor

GTM = Good to me 😃

@alexpozzi alexpozzi force-pushed the issue-54155/add-the-ability-to-confiure-cdata-wrapper-pattern branch from 43e0750 to 129bb3f Compare April 29, 2024 11:40
@alexpozzi alexpozzi force-pushed the issue-54155/add-the-ability-to-confiure-cdata-wrapper-pattern branch 2 times, most recently from 8b5f95c to 8edf532 Compare April 30, 2024 10:26
@alexpozzi alexpozzi force-pushed the issue-54155/add-the-ability-to-confiure-cdata-wrapper-pattern branch from 8edf532 to 8ab57d1 Compare April 30, 2024 15:28
@nicolas-grekas nicolas-grekas modified the milestones: 7.2, 7.1 May 2, 2024
@fabpot
Copy link
Member

fabpot commented May 2, 2024

Thank you @alexpozzi.

@fabpot fabpot merged commit 03e0318 into symfony:7.1 May 2, 2024
6 of 10 checks passed
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.

[Serializer] Add XmlEncoder::CDATA_WRAPPING_PATTERN context option
6 participants