Skip to content

fix: parse empty/whitspace only doctype internal subset#692

Merged
karfau merged 4 commits intoxmldom:masterfrom
boshold:bugfix/691
Jul 28, 2024
Merged

fix: parse empty/whitspace only doctype internal subset#692
karfau merged 4 commits intoxmldom:masterfrom
boshold:bugfix/691

Conversation

@kboshold
Copy link
Contributor

@kboshold kboshold commented Jul 17, 2024

Added a condition to the parser to allow empty doctypes

@kboshold kboshold changed the title Fixes #691 fix: Fixes #691 Jul 17, 2024
kboshold added 2 commits July 17, 2024 09:25
@kboshold
fix: Fixes #691
532a35c 
Added a condition to the parser to allow empty doctypes
@kboshold
fix: Removed unused code
cdf136b
@kboshold kboshold mentioned this pull request Jul 17, 2024
Closed
@codecov
Copy link

codecov bot commented Jul 21, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.28%. Comparing base (cbe6e21) to head (b12a09b).

Additional details and impacted files 
@@            Coverage Diff             @@
##           master     #692      +/-   ##
==========================================
+ Coverage   94.27%   94.28%   +0.01%     
==========================================
  Files           8        8              
  Lines        2096     2100       +4     
  Branches      537      538       +1     
==========================================
+ Hits         1976     1980       +4     
  Misses        120      120

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

@karfau karfau changed the title fix: Fixes #691 fix: parse empty doctype decl correctly Jul 21, 2024
@karfau
Copy link
Member

karfau commented Jul 21, 2024

Thank you for your contribution.
If the doctype decl would be [] the internalSubset would be an empty string, right?
Do you think it is worth adding another test case for that?

Once the formatting issue is fixed, I can land it.

@kboshold
Copy link
Contributor Author

kboshold commented Jul 22, 2024 •
edited
Loading

@karfau I added the test: https://github.com/xmldom/xmldom/pull/692/files#diff-7d53de1cebcbf7f6d753bd87ec5bd039dab2e818c406d35439c952419c1a2e42R125

Would be cool if you could create a beta12 afterwards!

@karfau
Copy link
Member

karfau commented Jul 22, 2024

Thx for adding the test.
Can you also fix the prettier issue by running npm run format and commiting and pushing the resulting changes?
Thx.

I will try to release a new version soon, potentially even 0.9.0

@kboshold
Copy link
Contributor Author

kboshold commented Jul 22, 2024

@karfau I fixed the formatting.

BTW (Not replated to the PR but for 0.9.0): There is currently an typing issue with the DOMImplementation - Its definied as a type but need to bedefined as a class to use the "new" keyword

@karfau
Copy link
Member

karfau commented Jul 22, 2024

@karfau I fixed the formatting.

Thx

BTW (Not replated to the PR but for 0.9.0): There is currently an typing issue with the DOMImplementation - Its definied as a type but need to bedefined as a class to use the "new" keyword

Thx for the information. Could you file a new issue for that?

karfau
karfau reviewed Jul 22, 2024
View reviewed changes
@karfau
Copy link
Member

karfau commented Jul 26, 2024 •
edited
Loading

@kpalatzky for some reason I'm not able to push to the branch that you created to provide the changes.
Can you please enable the option for xmldom maintainers to push to this PR or apply the following commit to it?
(Alternatively I would be able to create another PR to add those changes, but I would prefer to land this one.)

Update: I decided to land this PR and open a follow up for the refactoring, to avoid more waiting and delays before the upcoming release.

@karfau karfau linked an issue Jul 26, 2024 that may be closed by this pull request
Doctypes without children are broken #691
Closed
@karfau karfau added this to the 0.9.0 milestone Jul 26, 2024
@karfau karfau changed the title fix: parse empty doctype decl correctly fix: parse empty/whitspace only doctype internal subset Jul 28, 2024
@karfau karfau merged commit 88481e8 into xmldom:master Jul 28, 2024
@karfau karfau mentioned this pull request Jul 28, 2024
Merged
karfau added a commit that referenced this pull request Jul 28, 2024
@karfau
refactor: merge redundant ']' condition at start of while loop (#698)
d771cb9 
with the fix added in #692 `parseDoctypeInternalSubset` has two places
that check for the ending symbol and returned the `internalSubSet` as a
string. By moving it to the beginning of the while loop, we only need to
check the condition once.

I also added tests for more cases ending with a space before the closing
square bracket.
@vasmiheich vasmiheich mentioned this pull request Sep 26, 2024
Open
@noushiyo noushiyo mentioned this pull request Sep 26, 2024
Open
This was referenced Sep 26, 2024
Closed
Closed
@idleberg idleberg mentioned this pull request Sep 26, 2024
Closed
@Soontao Soontao mentioned this pull request Sep 26, 2024
Open
@deepakprabhakara deepakprabhakara mentioned this pull request Sep 27, 2024
Closed
@ITSecOps-404 ITSecOps-404 mentioned this pull request Oct 5, 2024
Open
@1AhmedYasser 1AhmedYasser mentioned this pull request Oct 27, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@karfau karfau karfau approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

0.9.0

Development

Successfully merging this pull request may close these issues.

Doctypes without children are broken

2 participants

@kboshold @karfau