-
-
Notifications
You must be signed in to change notification settings - Fork 310
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
Decode HTML entities in text values #40
Comments
parse html entities in text values, fixes #40
Published |
I believe it should not parse HTML entities for CDATA. |
@amitguptagwl CDATA also can contain HTML entities https://en.wikipedia.org/wiki/CDATA |
Yes. :)
But what I'm getting from current parser is
Expectation 2:
Expectation 3:
|
And it's right I placed holywar at work ) left angle brackets and ampersands may occur in their literal form; they need not (and cannot) be escaped using " < " and " & " but no any word about other entities, and even not specified correctly NEED NOT & CAN NOT is not the same as MUST NOT For example CDATA can contain HTML parts
For consistensy You can disable parse CDATA with this or make it optional. But it need more refactor due in code as I know you join it together |
As per the wiki,
Above phrase in my words (understanding),
|
@amitguptagwl I will make some refactoring, and place PR tomorrow |
Thanks @Delagen . In case if you are busy with some other prior work, we can just roll back the changes for the time being. And will handle them later. I'm anyhow planning to rewrite parser but it'll take at least a month 😸 |
Thanks @amitguptagwl for work, I placed PR to not decode CDATA
result in:
I know it rare case, but I am sure it must be Example: <a>a
<![CDATA[asdf&]]>&<![CDATA[asdf]]>
a</a> { "a": "a\nasdf&&asdf\na" } |
Sorry I'm holding this PR as I'm rewriting the parser So it'll become to handle this situation, large files, reduce time for separate validation etc. I'm 60% complete with the change. Will update you with the progress. |
Thanks for your project. I have a pleasure to contribute if project owner interested in making it code better. ) |
v3 is live to handle this issue |
Checklist
I include PR soon
The text was updated successfully, but these errors were encountered: