-
-
Notifications
You must be signed in to change notification settings - Fork 83
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
add recursion limit of 500 for DTD parsing #159
Conversation
@pjfanning Excellent, thank you for providing this! The only thing I was wondering is whether this could be made configurable? I know 500 should typically be enough for real usage so maybe it's not a big deal -- and/or could add configurability if someone actually hits the limit -- but then again other processing limits are settable so there should be an existing pattern to follow. Although it may be (I didn't check the code) that perhaps config object is not being passed to validator. Another question: if it's easy enough to rebase, could you rebase this against |
2302c38
to
3990f6b
Compare
@cowtowncoder I changed the target branch to 5.3. I added a static |
Ok looks good: I will change configuration to use |
@pjfanning Ok, created #160 as the issue for PR, and changed configuration to be applied similar to all other Woodstox-specific properties (via Planning to release 5.4.0 and 6.4.0 (minor version upgrade since it's an API change even if minor (I guess one could argue that adding a property could be done in patch but whatever)). |
(cherry picked from commit 7e93907)
Relates to #157
It's DTD parsing where the recursion happens and who really wants to parse DTDs any more, let alone deeply nested ones? So it seems adequate to just set a recursion limit instead of trying to rewrite the code not to recurse (and the extensive rewrite that would need).
With testing, I've tried a few of the files that supposedly cause the recursion issue but I all I get is UTF-8 parsing issues - so the code never even really gets to the recursion.
I modified the first XML file in ...5219006592450560.zip and removed the UTF-8 issues - this has given me a test case that does fail with the recursion limit kicking in.