Community
Participate
Working Groups
Created attachment 117942 [details] artifacts.xml with duplicate elements I was creating some invalid xml files for use in some test cases I'm writing and I noticed that if I include a duplicate element (such as a second <mappings> for an artifact repository) the parser treats the duplicate's closing statement (</mappings>) as the end of the parse. Using the attached artifacts.xml you can see that the file is parsed properly up until the second declaration of <mappings>. This element is correctly identified as a duplicate element. Similarly, the following 3 "rule" elements are properly recognized as invalid. However, noticed that the parse then exits before reaching the "artifacts" element immediately after the </mappings> without any errors. A similar issue is if you remove the second "</mappings>" from the artifacts.xml, you receive the following error message: [Fatal Error] :32:3: The element type "mappings" must be terminated by the matching end-tag "</mappings>". However, in this case the "artifacts" element is parsed.
Upon reflection I think I should clarify my position on the error message when the duplicate element is not closed. First, I don't think the error message is accurate. To me, a fatal error means the program can't continue, where in this case it does. Second, and more importantly, I think this issue is most likely a side effect of duplicate closing elements not being handled correctly. This is why I included it in the bug report.
Created attachment 118166 [details] Potential fix (needs more testing) Since we are treating duplicate elements as a warning, we should add an IgnoringHandler that will just process and skip the duplicate element.
The patch seems to work fine for my purposes, but I agree that we'd need some more extensive testing. Also, this fixes the other issue of the missing closing element saying there was a fatal error and not failing. Now it gives the same error and throws a ProvisionException
Created attachment 118267 [details] Adds test case for a duplicate element Place the XML file I attached earlier in /testData/bug255401/ The test case verifies the patch by ensuring that the artifacts included in the XML (below the duplicate's closing element) are actually parsed. As for the missing closing on the duplicate element not actually causing a fatal error... I was wrong on that one. Turns out the exception was being caught in my code and I just didn't notice it. I have not included test cases for that issue.
I should add that John's patch modifies a part of the parser all other parsers inherit from, so I don't think we need to worry about testing this error on individual types of repositories.
Thanks Andrew. Assigning to myself to release the fix and tests.
Thanks Andrew, fix released. I merged the test into ArtifactRepositoryManagerTest rather than creating a new test class. Watch out for copyright statements (and other cruft) when you copy files... you added a new test class but the copyright was for different companies, which I assume came from another class.
Sorry about that. I'll be more careful in the future.
