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

Bug fix: guard against left recursion in DTDs. #10

Merged
merged 1 commit into from
Jun 5, 2020

Conversation

svenemtell
Copy link
Contributor

This is my suggested fix to the problem discussed in #9.
I don't have a good understanding of the function patched but have merely tried to copy the essence of the fix mentioned by ruricolist and made some personal tests.

@luismbo
Copy link
Member

luismbo commented May 24, 2020

@svenemtell could you add a test case as well?

@svenemtell
Copy link
Contributor Author

svenemtell commented May 24, 2020

@luismbo Is there a test suite that I can add to?
The tests in /test seem to look for the local path "/home/david/2001/...". Are the instructions regarding downloading test suites at https://common-lisp.net/project/cxml/installation.html#tests up to date?
I tried to use the test suite mentioned in #4 but got an error:

(cxml-tests:run-tests)

Running test suite CXML
 Running test suite XMLCONF
  Running test SAX valid/sa/012.xml: test applies to parsers without namespace support, skipping

Error: duplicate internal subset
  1 (continue) Skip this test
  2 Rerun the test #<IT.BESE.FIVEAM::TEST-CASE SAX 41110EB403>
  3 Signal an exceptional test failure and abort the test #<IT.BESE.FIVEAM::TEST-CASE SAX 41110EB403>.
  4 Ignore the rest of the tests and explain current results
  5 (abort) Return to top loop level 0.

@svenemtell
Copy link
Contributor Author

I forked the test suite from @ruricolist and added a test case svenemtell/cxml-tests@f4210da.

Running the test suite without this pull request produces a Stack overflow while running the test suite with the pull request doesn't produce a Stack overflow.

This is how I run the test suite in LispWorks Professional 7.1.2 64-bit on macOS:

(pushnew "/Volumes/source/" ql:*local-project-directories*)
(ql:quickload :cxml)
(ql:quickload :cxml-tests)
(cxml-tests:run-tests)

The output without the pull request
...
xmltest/valid/sa/120.xml [not validating:] input/output [validating:] FAILED:
Stack overflow (stack size 15998).
[
Left recursive.]
...
7/1842 tests failed; 744 tests were skipped.
...

The output with the pull request does not contain the Stack overflow
...
6/1842 tests failed; 744 tests were skipped.
...

@luismbo luismbo merged commit 8701da0 into sharplispers:master Jun 5, 2020
@luismbo
Copy link
Member

luismbo commented Jun 5, 2020

@svenemtell cool, maybe you'll want to submit your new test to cxml-tests and at some point someone should merge cxml-tests into this project, yes, no, maybe?

@svenemtell
Copy link
Contributor Author

svenemtell commented Jun 6, 2020

Great!
I added this pull request ruricolist/cxml-tests#1.
Is some work needed to have this fix in the next quicklisp dist, or is it done automatically?

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.

2 participants