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

Always use the system default TransformerFactory in FormatXmlHelper #2918

Merged
merged 2 commits into from
Jan 31, 2025

Conversation

ianprime0509
Copy link
Contributor

Closes #2911
Closes #2913

The implementation of FormatXmlHelper relies on a non-standard TransformerFactory attribute, indent-number, which is not supported by all implementations. By using TransformerFactory.newDefaultInstance instead of TransformerFactory.newInstance, the system default implementation (com.sun.org.apache.xalan.internal.xsltc.trax.TransformerFactoryImpl on OpenJDK-derived JDKs) will be used regardless of any other implementations which might be present on the classpath.


Unfortunately, I don't see any way to add or update any tests for this change without introducing a dependency on another XML library. To ensure this change has the expected behavior, I used the same reproduction steps as in #2911 (include Saxon-HE in a small test project): the exception thrown when invoking the WireMockServer constructor no longer appears when using a version with this change installed to my local Maven repository.

Another possible approach would be to try to unify this logic with

TransformerFactory transformerFactory;
try {
// Optimization to get likely transformerFactory directly, rather than going through
// FactoryFinder#find
transformerFactory =
(TransformerFactory)
Class.forName(
"com.sun.org.apache.xalan.internal.xsltc.trax.TransformerFactoryImpl")
.getDeclaredConstructor()
.newInstance();
} catch (Exception e) {
transformerFactory = TransformerFactory.newInstance();
}
transformerFactory.setAttribute("indent-number", 2);
, which appears to serve a similar purpose, but was written before Java 11 was the baseline (TransformerFactory.newDefaultInstance was introduced in Java 9). Let me know if you'd like me to try to consider that in this PR as well.

References

Submitter checklist

  • Recommended: Join WireMock Slack to get any help in #help-contributing or a project-specific channel like #wiremock-java
  • The PR request is well described and justified, including the body and the references
  • The PR title represents the desired changelog entry
  • The repository's code style is followed (see the contributing guide)
  • Test coverage that demonstrates that the change works as expected
  • For new features, there's necessary documentation in this pull request or in a subsequent PR to wiremock.org

@ianprime0509 ianprime0509 requested a review from a team as a code owner December 17, 2024 05:25
@welcor
Copy link

welcor commented Jan 16, 2025

Looks like the build failed due to a port conflict - it will probably run ok if triggered again.

@tomakehurst
Copy link
Member

Ugh, do you mind re-running Spotless and pushing?

The rollover to the new year is causing it to fail now.

Closes wiremock#2911
Closes wiremock#2913

The implementation of `FormatXmlHelper` relies on a non-standard `TransformerFactory` attribute, `indent-number`, which is not supported by all implementations. By using `TransformerFactory.newDefaultInstance` instead of `TransformerFactory.newInstance`, the system default implementation (`com.sun.org.apache.xalan.internal.xsltc.trax.TransformerFactoryImpl` on OpenJDK-derived JDKs) will be used regardless of any other implementations which might be present on the classpath.
@ianprime0509
Copy link
Contributor Author

Sure, no problem, I just rebased and ran Spotless on top to update the copyright year.

@ianprime0509
Copy link
Contributor Author

Hmm, it looks like just one of the Spotless jobs in CI is failing for copyright years on files which aren't being changed in this PR. Running Spotless on my end didn't touch these files, but I also use Linux while the failing CI job is using Windows. Should I update those files manually?

@tomakehurst
Copy link
Member

We were just discussing this. Look like a bug in the latest Spotless and we were considering disabling it for Windows for now, but if you don't mind making those changes it would definitely help move things forward.

@ianprime0509
Copy link
Contributor Author

Sounds good, I just added an extra commit on top which should address all the files that were failing in that CI run.

@com2ghz
Copy link

com2ghz commented Jan 29, 2025

With temurin-21 i am facing this exception when I try to upgrade from wiremock-standalone:3.9.1 to 3.10.0 and get this exception. Looks like this issue is describing this problem:

Caused by: java.lang.IllegalArgumentException: Unrecognized configuration feature: indent-number
	at net.sf.saxon.Configuration.setConfigurationProperty(Configuration.java:4373)
	at net.sf.saxon.jaxp.SaxonTransformerFactory.setAttribute(SaxonTransformerFactory.java:340)
	at com.github.tomakehurst.wiremock.extension.responsetemplating.helpers.FormatXmlHelper.<init>(FormatXmlHelper.java:68)
	at com.github.tomakehurst.wiremock.extension.responsetemplating.helpers.WireMockHelpers$30.<init>(WireMockHelpers.java:284)
	at com.github.tomakehurst.wiremock.extension.responsetemplating.helpers.WireMockHelpers.<clinit>(WireMockHelpers.java:283)

@tomakehurst tomakehurst merged commit bd17065 into wiremock:master Jan 31, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants