There's some discussion in the PR, but for this mailing list I wanted to comment that this is indeed a deep area of algorithmic complexity in the Daffodil runtime.
Whether the code is actually factored adequately such that it enables implementing the right DFDL behavior is not entirely clear, but it is encouraging that this fix seems to be a small change without requiring refactoring. I am sure there are other bugs in this general area of the code and getting everything exactly right so that every nuance of the DFDL spec is properly implemented is quite difficult and will require development of an extensive collection of tests that cover all these various combinations of features of DFDL. I think right now, the important thing is no regression on any known existing DFDL schemas. ________________________________ From: Carlson, Ian <icarl...@owlcyberdefense.com> Sent: Wednesday, October 21, 2020 6:04 PM To: dev@daffodil.apache.org <dev@daffodil.apache.org> Subject: 2399 - Changing error code for missing separators All, I’ve done a great deal of digging to find the cause of the bug DAFFODIL-2399. It appears to come down to SeparatedSequenceChildParseResultHelper.scala line 282. In this case, we have found an out of scope delimiter %NL; before finding any elements, and as a result we abort parsing the sequence. We sensibly set the return status to ParseAttemptStatus.MissingSeparator. However, since the requiredOptional field is true, we have a status of success. So far so good – everything works out until we roll back up to SequenceParserBases.scala, line 270. The MissingSeparator status again sets a status to success and terminates parsing of the sequence, but does not reset to a PoU. This is the reason our errors aren’t being discarded. My suggested solution is simply to replace ParseAttemptStatus.MissingSeparator with ParseAttemptStatus.AbsentRep. I bring this up to the mailing list because while this does seem to solve the problem without breaking any other tests, this is a very deep behavior in our parsing routine, and I want to see if anyone sees any potential dangers with the change. I’ve created a pull request: https://github.com/apache/incubator-daffodil/pull/444 with this change for review. [A picture containing object, clock Description automatically generated] Ian Carlson | Software Engineer [Owl Cyber Defense] W icarl...@owlcyberdefense.com<https://owlcyberdefense.com/> Connect with us! Facebook<https://www.facebook.com/owlcyberdefense/> | LinkedIn<https://www.linkedin.com/company/owlcyberdefense/> | Twitter<https://twitter.com/owlcyberdefense> [Find us at our next event. Click Here.]<https://owlcyberdefense.com/resources/events/?utm_source=owl-cyber-defense&utm_medium=email&utm_content=banner&utm_campaign=2020-events> The information contained in this transmission is for the personal and confidential use of the individual or entity to which it is addressed. If the reader is not the intended recipient, you are hereby notified that any review, dissemination, or copying of this communication is strictly prohibited. If you have received this transmission in error, please notify the sender immediately
