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

Fix NodeDescriptorImporterBase.fromString to handle non ascii chars in xml file #120

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

WolfgangHG
Copy link

@WolfgangHG WolfgangHG commented Feb 12, 2025

This fixes #119

Arqullian uses this code to parse "arquillian.xml": https://github.com/arquillian/arquillian-core/blob/main/config/impl-base/src/main/java/org/jboss/arquillian/config/impl/extension/ClasspathConfigurationPlaceholderResolver.java
It exports a descriptor to a string, replaces some placeholders and reimports the descriptor from this string.
This fails if "arquillian.xml" contains non-ascii chars (though the xml files defines "UTF-8" encoding).

This pull request adds an override fromString to NodeDescriptorImporterBase. In this method, I create a java.io.StringReader, which is passed to the new method NodeImporter.importAsNode(Reader), implemented in XmlDomNodeImporterImpl. Here, a org.xml.sax.InputSource is created using this Reader, and the DocumentBuilder parses this source.

The NodeDescriptorImporterBase base class DescriptorImporterBase still contains the previous implementation of fromString that calls fromStream(new ByteArrayInputStream(string.getBytes())). I would have preferred to add a fromReader to the base class too, but this would have meant adding a method to the interface org.jboss.shrinkwrap.descriptor.api.DescriptorImporter.

Things to improve - please give me feedback:

  • NodeDescriptorImporterBase: the existing fromStream and my new method fromString contain similar code. The part after the comment "// Create the end-user view" could be extracted to a helper method.
  • same applies to XmlDomNodeImporterImpl.importAsNode(Reader).
  • I didn't add a test. Actually, I don't have an idea how to implement one - probably I would have to do something similar to the arquillian code and load the arquillian descriptor from a file, convert to a string and re-parse it from this string? But currently, the test suite does not run for me - probably because I use Java 1.8 and it requires older versions? I saw that there is a pull request to switch to Java 1.8 - maybe this one fixes the test suite.

I tested it in my sample project and will attach a ut8 "arquillian.xml" and a "windows-1252" file (both containing umlauts in a comment) to #119. Both files did not show the failure from the initial bugreport.

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.

Parse error in "arquillian.xml" with non-ascii chars
1 participant