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

Fix '<' or '>' in DTD comment throwing an error. #533

Merged

Conversation

Cwazywierdo
Copy link
Contributor

Purpose / Goal

This PR fixes a bug where if a comment in the doctype definition contains '<' or '>' it throws an error. The XML specification allows '<' and '>' in comments, and does not note an exception excluding them in the DTD.

Closes #530

Benchmarks

Before

fxp v3 : 47776.15215500227 requests/second
fxp : 30512.79096797974 requests/second
fxp - preserve order : 33437.923151138326 requests/second
xmlbuilder2 : 10620.38965853095 requests/second
xml2js  : 8749.365823529228 requests/second

After

fxp v3 : 47832.232365539996 requests/second
fxp : 30937.34839987414 requests/second
fxp - preserve order : 33804.982961890804 requests/second
xmlbuilder2 : 10637.245964526057 requests/second
xml2js  : 8782.803299604513 requests/second

Type

Please mention the type of PR

  • [x]Bug Fix
  • [ ]Refactoring / Technology upgrade
  • [ ]New Feature

Note : Please ensure that you've read contribution guidelines before raising this PR. If your PR is in progress, please prepend [WIP] in PR title. Your PR will be reviewed when [WIP] will be removed from the PR title.

Bookmark this repository for further updates.

@coveralls
Copy link

coveralls commented Jan 22, 2023

Coverage Status

Coverage: 98.511% (+0.04%) from 98.467% when pulling 58bd746 on Cwazywierdo:doctype-comment-angle-bracket into 007d638 on NaturalIntelligence:master.

@coveralls
Copy link

Coverage Status

Coverage: 98.511% (+0.04%) from 98.467% when pulling 58bd746 on Cwazywierdo:doctype-comment-angle-bracket into 007d638 on NaturalIntelligence:master.

@amitguptagwl
Copy link
Member

I would be checking this PR soon.

@amitguptagwl
Copy link
Member

can you please merge your change to dev branch?

@Cwazywierdo Cwazywierdo changed the base branch from master to dev January 22, 2023 18:06
@Cwazywierdo
Copy link
Contributor Author

Did it, I think.

@Cwazywierdo Cwazywierdo changed the base branch from dev to master January 22, 2023 18:26
@Cwazywierdo Cwazywierdo changed the base branch from master to dev January 22, 2023 18:28
@amitguptagwl amitguptagwl merged commit 30624d7 into NaturalIntelligence:dev Jan 25, 2023
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.

Error when using '<' and '>' in comment in doctype markup declaration list
3 participants