tuxji commented on pull request #560: URL: https://github.com/apache/daffodil/pull/560#issuecomment-848790387
+1 I reviewed the three new commits. First and third commits indeed address all my comments and the changes look great. Second commit has a slight problem with test coverage. Your @Test def testXMLTextInfosetInputterDisallowsDocType() tried several ways to load XML securely with the Woodstox XML library and found the best way was to both turn off features and add a doctype check / throw exception in XMLTextInfosetInputter.scala. I believe you then got inspired to add a doctype check / throw exception to a similar place in JDOMInfosetInputter.scala but that line doesn't get called by any test either because we either need a new test to check JDOM secure loading or JDOM doesn't let programs see the doctype at all when you enable secure XML loading. If the latter is the case, It's probably a good idea to keep the check in case of malicious input and just add a comment saying so along with comments to disable codecov. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected]
