ukf / ukf-testbed

UK federation tooling testbed
Apache License 2.0
1 stars 1 forks source link

Hitting the JDK 11 XML processing limits #21

Closed philsmart closed 5 months ago

philsmart commented 10 months ago

The check_reqattr ruleset hits the JDK11 and onward XML processing limits. I think we will need to adjust those in the upstream mda-validator.

iay commented 10 months ago

Can you drop a log in here? It looks like some of the limits can be modified by passing in transformer parameters, which the MDA Stage allows already. The other one looks like a system parameter and that might need some upstream support.

philsmart commented 10 months ago

This is the stack trace (you might mean something different, just let me know)

Caused by: org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'check_reqattr' defined in class path resource [default-validator.xml]: Invocation of init method failed; nested exception is net.shibboleth.utilities.java.support.component.ComponentInitializationException: XSL transformation engine misconfigured
    at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.initializeBean(AbstractAutowireCapableBeanFactory.java:1804) ~[spring-beans-5.3.27.jar:5.3.27]
    at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.doCreateBean(AbstractAutowireCapableBeanFactory.java:620) ~[spring-beans-5.3.27.jar:5.3.27]
    at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBean(AbstractAutowireCapableBeanFactory.java:542) ~[spring-beans-5.3.27.jar:5.3.27]
    at org.springframework.beans.factory.support.AbstractBeanFactory.lambda$doGetBean$0(AbstractBeanFactory.java:335) ~[spring-beans-5.3.27.jar:5.3.27]
    at org.springframework.beans.factory.support.DefaultSingletonBeanRegistry.getSingleton(DefaultSingletonBeanRegistry.java:234) ~[spring-beans-5.3.27.jar:5.3.27]
    at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:333) ~[spring-beans-5.3.27.jar:5.3.27]
    at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:208) ~[spring-beans-5.3.27.jar:5.3.27]
    at org.springframework.beans.factory.support.BeanDefinitionValueResolver.resolveReference(BeanDefinitionValueResolver.java:330) ~[spring-beans-5.3.27.jar:5.3.27]
    ... 75 common frames omitted
Caused by: net.shibboleth.utilities.java.support.component.ComponentInitializationException: XSL transformation engine misconfigured
    at net.shibboleth.metadata.dom.AbstractXSLProcessingStage.doInitialize(AbstractXSLProcessingStage.java:308) ~[aggregator-pipeline-0.9.2.jar:na]
    at net.shibboleth.utilities.java.support.component.AbstractInitializableComponent.initialize(AbstractInitializableComponent.java:65) ~[java-support-8.3.1.jar:na]
    at jdk.internal.reflect.GeneratedMethodAccessor4.invoke(Unknown Source) ~[na:na]
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[na:na]
    at java.base/java.lang.reflect.Method.invoke(Method.java:568) ~[na:na]
    at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.invokeCustomInitMethod(AbstractAutowireCapableBeanFactory.java:1930) ~[spring-beans-5.3.27.jar:5.3.27]
    at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.invokeInitMethods(AbstractAutowireCapableBeanFactory.java:1872) ~[spring-beans-5.3.27.jar:5.3.27]
    at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.initializeBean(AbstractAutowireCapableBeanFactory.java:1800) ~[spring-beans-5.3.27.jar:5.3.27]
    ... 82 common frames omitted
Caused by: javax.xml.transform.TransformerConfigurationException: JAXP0801002: the compiler encountered an XPath expression containing '101' operators that exceeds the '100' limit set by 'FEATURE_SECURE_PROCESSING'.
    at java.xml/com.sun.org.apache.xalan.internal.xsltc.trax.TransformerFactoryImpl.newTemplates(TransformerFactoryImpl.java:1079) ~[na:na]
    at net.shibboleth.metadata.dom.AbstractXSLProcessingStage.doInitialize(AbstractXSLProcessingStage.java:305) ~[aggregator-pipeline-0.9.2.jar:na]
    ... 89 common frames omitted
iay commented 10 months ago

That looks like the one they say can be adjusted by properties on the transformer factory:

For the XSLT processor, the properties can be changed through the TransformerFactory. For example,

        TransformerFactory factory = TransformerFactory.newInstance();

        factory.setAttribute("jdk.xml.xpathTotalOpLimit", "1000");

Unfortunately, although the MDA includes a mechanism to set parameters on the transform itself, it doesn't go via a TransformerFactory so I'm not sure if we can inject this kind of thing specifically (there are multiple independent sets of random values you can inject in different places). I'll look into it a bit more.

iay commented 10 months ago

I take that back, it looks like I included this as well. You'd probably need to add a transformerAttributes property to the stage (untested):

<bean ..>
    <property name="transformerAttributes">
        <map>
            <entry key="jdk.xml.xpathTotalOpLimit" value="0"/>
        </map>
    </property>
</bean>

If that seems to work, we should consider making it easier to access somehow.

philsmart commented 10 months ago

Excellent, Thanks. I will try this out tomorrow.

philsmart commented 10 months ago

I can confirm this works. Although strangely it requires the attribute jdk.xml.xpathExprOpLimit which just controls the number of operators an XPath expression can contain, whereas jdk.xml.xpathTotalOpLimit controls the number of XPath operators in an XSL Stylesheet—which looks more correct to me, given the operators I presume are coming from the check_reqatr.xsl stylesheet.

<property name="transformAttributes">
            <map>
                <entry key="jdk.xml.xpathExprOpLimit" value="0"/>
            </map>
        </property>
philsmart commented 10 months ago

I will add this to some wiki page for reference.

philsmart commented 10 months ago

https://github.com/ukf/ukf-testbed/wiki/Native-JDK-XSLT-Processing-Gotcha's

philsmart commented 10 months ago

Actually, adding the jdk.xml.xpathExprOpLimit breaks the Xalan factory:

Caused by: java.lang.IllegalArgumentException: Not supported: jdk.xml.xpathExprOpLimit
    at org.apache.xalan.processor.TransformerFactoryImpl.setAttribute(TransformerFactoryImpl.java:576) ~[xalan-2.7.1.jar:na]
    at net.shibboleth.metadata.dom.AbstractXSLProcessingStage.doInitialize(AbstractXSLProcessingStage.java:277) ~[mda-framework-0.10.0-SNAPSHOT.jar:na]
    at net.shibboleth.shared.component.AbstractInitializableComponent.initialize(AbstractInitializableComponent.java:62) ~[shib-support-9.0.0-SNAPSHOT.jar:na]
    at jdk.internal.reflect.GeneratedMethodAccessor15.invoke(Unknown Source) ~[na:na]
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[na:na]
    at java.base/java.lang.reflect.Method.invoke(Method.java:568) ~[na:na]
    at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.invokeCustomInitMethod(AbstractAutowireCapableBeanFactory.java:1869) ~[spring-beans-6.0.10.jar:6.0.10]
    at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.invokeInitMethods(AbstractAutowireCapableBeanFactory.java:1826) ~[spring-beans-6.0.10.jar:6.0.10]
    at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.initializeBean(AbstractAutowireCapableBeanFactory.java:1766) ~[spring-beans-6.0.10.jar:6.0.10]
    ... 82 common frames omitted

So we can not add this to a Xalan enabled validator.

philsmart commented 5 months ago

This was fixed, but the other way round than I thought. That is, the check_reqattr bean in default-validator-beans.xml contains the attribute setting for jdk.xml.xpathExprOpLimit, and the 10x and 9x branches override that bean to remove that attribute. So the non-xalan branches work with the default beans, and the xalan branches override it.

Given we currently run Xalan, maybe this should be the other way round. That is, the Xalan branches work with the default beans, and the non-xalan branches add the attribute.

iay commented 5 months ago

This was fixed, but the other way round than I thought.

OK, that mostly explains why I couldn't see why it was working 😉 although I should maybe have grepped harder.

maybe this should be the other way round

It's arguable either way, but on balance I agree. My thinking would be that at any given time if at all possible the configuration that matches whatever we're currently doing in production should be what's described in the default beans. That way, there's more certainty that we're testing what we're really running and when we move from one production configuration to the next, we can see by looking at the overrides for the new configuration what we need to change in the production deployment.

I think I've mentioned that my medium term plan involves actually acquiring the current production configuration directly from the ukf-meta repository so that we're sure about what we're testing. That's not yet, but it's much closer to possible if that configuration is the one without overrides.

So, let's reverse it, re-test and then close this when that's done.

philsmart commented 5 months ago

Agreed. I will do that.

philsmart commented 5 months ago

Reversing that works fine. I am thinking of pushing to the main branch so the other PRs we have can check conflicts with it before they are merged in (they should not overlap). @iay what do you think?

iay commented 5 months ago

Sounds simple enough, I'd say go ahead.