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

TCK Challenge -- Incorrect requirement around combination of 'length' and 'minLength' facets #82

Closed
bstansberry opened this issue Oct 26, 2023 · 16 comments · Fixed by #83
Labels
accepted Accepted TCK challenge challenge TCK challenge

Comments

@bstansberry
Copy link

bstansberry commented Oct 26, 2023

Summary

Two TCK tests are requiring behavior related to the relationship between 'length' and 'minLength' facets on a simple type that go beyond what XML Schema requires. They expect the JAXB implementation to reject compilation of xsds that are valid. Implementors could pass these tests for many years due to bugs in Xerces or Java SE's internal adaption of xerces, but since January 2022 Xerces and the various SE releases have been rolling out fixes for that bug and the tests will not pass using the latest Xerces or SE releases.

The Challenge

The two tests in question are negative tests that expect the implementation to fail to compile the test xsd. The tests both use an equivalent xsd.

  1. xml_schema/msData/datatypes/Facets/Schemas/jaxb/IDREFS_length006_395.html#IDREFS_length006_395

The xsd is as follows:

https://github.com/jakartaee/jaxb-tck/blob/master/xml_schema/tests/xml_schema/msData/datatypes/Facets/Schemas/IDREFS_length006.xsd

<?xml version='1.0'?>
<xsd:schema xmlns:xsd='http://www.w3.org/2001/XMLSchema' >
<!-- Schema to test IDREFS datatype -->
  <xsd:element name='test' type='fooType' />
  <xsd:complexType name='fooType' > 
    <xsd:sequence>
      <xsd:element name='foo' >
        <xsd:complexType>
          <xsd:simpleContent>
            <xsd:extension base="xsd:string">
              <xsd:attribute name='attrTest'>
                <xsd:simpleType> 
                  <xsd:restriction base="xsd:IDREFS">
                            <xsd:length value="5"/>
                            <xsd:minLength value="1"/>
                  </xsd:restriction>
                </xsd:simpleType>
              </xsd:attribute>
            </xsd:extension>
          </xsd:simpleContent>
        </xsd:complexType>
      </xsd:element>
    </xsd:sequence>
  </xsd:complexType>
</xsd:schema>
  1. xml_schema/msData/datatypes/Facets/Schemas/jaxb/NMTOKENS_length006_438.html#NMTOKENS_length006_438

The xsd used is :

https://github.com/jakartaee/jaxb-tck/blob/master/xml_schema/tests/xml_schema/msData/datatypes/Facets/Schemas/IDREFS_length006.xsd

The xsd content is the same as the previous one so I won't repost it.

Both are based on and restrict the IDREFs type:

http://www.w3.org/2001/XMLSchema

<xs:simpleType name="IDREFS" id="IDREFS">
  <xs:annotation>
    <xs:appinfo>
      <hfp:hasFacet name="length"/>
      <hfp:hasFacet name="minLength"/>
      <hfp:hasFacet name="maxLength"/>
      <hfp:hasFacet name="enumeration"/>
      <hfp:hasFacet name="whiteSpace"/>
      <hfp:hasFacet name="pattern"/>
      <hfp:hasProperty name="ordered" value="false"/>
      <hfp:hasProperty name="bounded" value="false"/>
      <hfp:hasProperty name="cardinality" value="countably infinite"/>
      <hfp:hasProperty name="numeric" value="false"/>
    </xs:appinfo>
    <xs:documentation source="http://www.w3.org/TR/xmlschema-2/#IDREFS"/>
  </xs:annotation>
  <xs:restriction>
    <xs:simpleType>
      <xs:list itemType="xs:IDREF"/>
    </xs:simpleType>
    <xs:minLength value="1" id="IDREFS.minLength"/>
  </xs:restriction>
</xs:simpleType>

In effect the test xsds add to the base type restriction that the list minLength is 1 an additional restriction that the list length must be 5. The xsds also repeat the base type's restriction that minLength = 1.

The tests expect compilation to fail. Perhaps this was because at some point the thinking was combining 'length' and 'minLength' was invalid. I don't know. But the XML Schema 2: Datatypes document lists the requirements around 'length' facets:

https://www.w3.org/TR/xmlschema-2/#rf-facets

4.3.1.4 Constraints on length Schema Components

Schema Component Constraint: length and minLength or maxLength

If length is a member of {facets} then

1. It is an error for minLength to be a member of {facets} unless
  1.1 the {value}] of minLength <= the {value} of length and
  1.2 there is type definition from which this one is derived by one or more restriction steps in which minLength has the same {value} and length is not specified.
2 It is an error for maxLength to be a member of {facets} unless
  2.1 the {value} of length <= the {value} of maxLength and
  2.2 there is type definition from which this one is derived by one or more restriction steps in which maxLength has the same {value} and length is not specified.

Schema Component Constraint: length valid restriction

It is an ·error· if length is among the members of {facets} of {base type definition} and {value} is not equal to the {value} of the parent length.

Neither of the TCK xsds shown above violate these constraints, so there is no reason they should fail to compile.

Background Info -- the Xerces/Java SE bug

Until recently JAXB implementations may not have been aware that the test was making an invalid assertion because of a bug in Xerces and Java SE that caused compilation of the test xsds to fail (and thus the negative tests to pass):

https://issues.apache.org/jira/browse/XERCESJ-1729

This was fixed in Xerces 2.12.2, released in January 2022. Following that OpenJDK began porting the Xerces 2.12.2 fixes into Java SE (for SE 19):

https://bugs.openjdk.org/browse/JDK-8282280

Backports to other SE branches were done; the complete list is shown in JDK-8282280. For the LTS SE streams it was resolved in October 2022 in SE 11.0.17 (https://bugs.openjdk.org/browse/JDK-8290810) and in SE 17.0.5 (https://bugs.openjdk.org/browse/JDK-8290508).

As test environments move to more recent Xerces or Java SE releases I expect they will start to fail these tests.

@bstansberry
Copy link
Author

Note that I don't think these tests can be 'fixed' in a patch release. Changing them to a positive test would break anyone who currently passes them, and changing them to test some other invalid relationship between length and minLength would be a new test.

@bstansberry
Copy link
Author

bstansberry commented Oct 26, 2023

Note that the data in the most recent TCK run for jaxb-ri at https://ci.eclipse.org/jaxb-impl/job/jaxb-jakarta-ri-tck/18/ indicates the test passes because of the Xerces or JDK bug. From IDREF_length006_525_IDREF_length006_525.jtr

#section:compile.xsd
----------messages:(1/1140)----------
command: javasoft.sqe.javatest.lib.ProcessCommand JAVA_HOME=/opt/tools/java/openjdk/jdk-11/latest JAXB_HOME=/home/jenkins/agent/workspace/jaxb-jakarta-ri-tck/jaxb-ri CLASSPATH=/home/jenkins/agent/workspace/jaxb-jakarta-ri-tck/xml-binding-tck/classes:/home/jenkins/agent/workspace/jaxb-jakarta-ri-tck/checker.jar:/home/jenkins/agent/workspace/jaxb-jakarta-ri-tck/jaxb-ri/mod/jakarta.xml.bind-api.jar:/home/jenkins/agent/workspace/jaxb-jakarta-ri-tck/jaxb-ri/mod/jaxb-impl.jar:/home/jenkins/agent/workspace/jaxb-jakarta-ri-tck/jaxb-ri/mod/jaxb-jxc.jar:/home/jenkins/agent/workspace/jaxb-jakarta-ri-tck/jaxb-ri/mod/jakarta.activation-api.jar:/home/jenkins/agent/workspace/jaxb-jakarta-ri-tck/xml-binding-tck/lib/javatest.jar /bin/sh /home/jenkins/agent/workspace/jaxb-jakarta-ri-tck/xml-binding-tck/linux/bin/xjc.sh -p javasoft.sqe.tests.datatypes.facets.schemas.idref_length006 -d /home/jenkins/agent/workspace/jaxb-jakarta-ri-tck/batch-multiJVM/work/classes/datatypes/facets/schemas/idreflength006 /home/jenkins/agent/workspace/jaxb-jakarta-ri-tck/xml-binding-tck/tests/xml_schema/msData/datatypes/Facets/Schemas/jaxb/../IDREF_length006.xsd
----------out1:(2/46)----------
parsing a schema...
Failed to parse a schema.
----------out2:(4/1321)----------
/opt/tools/java/openjdk/jdk-11/latest/bin/java -cp /home/jenkins/agent/workspace/jaxb-jakarta-ri-tck/xml-binding-tck/classes:/home/jenkins/agent/workspace/jaxb-jakarta-ri-tck/checker.jar:/home/jenkins/agent/workspace/jaxb-jakarta-ri-tck/jaxb-ri/mod/jakarta.xml.bind-api.jar:/home/jenkins/agent/workspace/jaxb-jakarta-ri-tck/jaxb-ri/mod/jaxb-impl.jar:/home/jenkins/agent/workspace/jaxb-jakarta-ri-tck/jaxb-ri/mod/jaxb-jxc.jar:/home/jenkins/agent/workspace/jaxb-jakarta-ri-tck/jaxb-ri/mod/jakarta.activation-api.jar:/home/jenkins/agent/workspace/jaxb-jakarta-ri-tck/xml-binding-tck/lib/javatest.jar com.sun.jaxb_tck.lib.JaxbCommand -xjc com.sun.jaxb_tck.lib.SchemaCompiler - -p javasoft.sqe.tests.datatypes.facets.schemas.idref_length006 -d /home/jenkins/agent/workspace/jaxb-jakarta-ri-tck/batch-multiJVM/work/classes/datatypes/facets/schemas/idreflength006 /home/jenkins/agent/workspace/jaxb-jakarta-ri-tck/xml-binding-tck/tests/xml_schema/msData/datatypes/Facets/Schemas/jaxb/../IDREF_length006.xsd
[ERROR] length-minLength-maxLength.1.1: For type #AnonType_attrTestfoofooType, it is an error for the value of length '-1' to be less than the value of minLength '1'.
  line 13 of file:/home/jenkins/agent/workspace/jaxb-jakarta-ri-tck/xml-binding-tck/tests/xml_schema/msData/datatypes/Facets/Schemas/IDREF_length006.xsd

result: Failed. exit code 1


test result: Passed. compilation failed as expected

Looking up the use of the message code that produced the '[ERROR] length-minLength-maxLength.1.1 ...' logging leads to the code corrected by XERCES-1729 and the Java SE ports of it. The "value of length '-1'" bit indicates that the code was reading the 'length' from the base IDREFs type, which doesn't configure length, so the value is -1. Reading that value when it wasn't defined is the XERCES-1729 bug.

@lukasj lukasj added challenge TCK challenge accepted Accepted TCK challenge labels Oct 27, 2023
@lukasj
Copy link
Contributor

lukasj commented Oct 31, 2023

I believe these tests should be excluded in TCKs for EE <=10 and changed to not expect failure for EE 11 (given EE 11 requires SE 21+)

@bstansberry
Copy link
Author

Thanks, @lukasj!

@scottmarlow
Copy link
Contributor

I believe these tests should be excluded in TCKs for EE <=10

+1000

@scottmarlow
Copy link
Contributor

scottmarlow commented Nov 8, 2023

@lukasj since this TCK challenge is accepted could you please close this issue as per TCK Process:

Accepted Challenges

A consensus that a test produces invalid results will result in the exclusion of that test from certification requirements, and an immediate update and release of an official distribution of the TCK including the new exclude list. The associated challenge issue MUST be closed with an accepted label to indicate it has been resolved.
.
.
.

If you do intend to release updated TCKs that could be worth keeping this open a bit longer as ^ is a bit ambiguous.

@lukasj
Copy link
Contributor

lukasj commented Nov 8, 2023

right, someone has to do the work and actually exclude the test... I've just haven't found a way how to do it yet. Do you know?

@scottmarlow
Copy link
Contributor

scottmarlow commented Nov 8, 2023

right, someone has to do the work and actually exclude the test... I've just haven't found a way how to do it yet. Do you know?

Do we have a CI job yet for running the TCK (build) make files? If not, I think a user workaround of updating the lib/jaxb_tck40.jtx file to contain the following might be enough:

xml_schema/msData/datatypes/Facets/Schemas/jaxb/IDREFS_length006_395.html#IDREFS_length006_395
xml_schema/msData/datatypes/Facets/Schemas/jaxb/NMTOKENS_length006_438.html#NMTOKENS_length006_438

Is https://ci.eclipse.org/jaxb where such jobs should be setup?

@lukasj
Copy link
Contributor

lukasj commented Nov 9, 2023

Is https://ci.eclipse.org/jaxb where such jobs should be setup?

Guru has them under platform CI, I think. Let's wait for him.

@lukasj
Copy link
Contributor

lukasj commented Nov 13, 2023

@gurunrao can you build the tck 4.0.1 from master, please? Thx

@scottmarlow
Copy link
Contributor

@gurunrao can you build the tck 4.0.1 from master, please? Thx

As per #83 (comment) feedback from @gurunrao, I should be able to build it via https://ci.eclipse.org/jakartaee-tck/job/10/job/eftl-jaxb-tck-build-run-100 and will do so now to stage the 3.0.1 TCK.

@scottmarlow
Copy link
Contributor

scottmarlow commented Nov 13, 2023

https://www.eclipse.org/downloads/download.php?file=/ee4j/jakartaee-tck/jakartaee10/staged/eftl/jakarta-xml-binding-tck-4.0.1.zip is the staged TCK that implementations can test against for validation.

@scottmarlow
Copy link
Contributor

WildFly passed against the staged https://www.eclipse.org/downloads/download.php?file=/ee4j/jakartaee-tck/jakartaee10/staged/eftl/jakarta-xml-binding-tck-4.0.1.zip TCK and we got the expected number of tests run Pass: 24,624 Fail: 0 Error: 0 Not-Run: 0

Previously we passed 24,624 tests but failed the two excluded tests. Thank @lukasj + @gurunrao!

@lukasj would you like me to create the https://github.com/jakartaee/specifications/issues for promoting the staged TCK?

@lukasj
Copy link
Contributor

lukasj commented Nov 13, 2023

@scottmarlow feel free to do so, thanks!

@scottmarlow
Copy link
Contributor

scottmarlow commented Nov 13, 2023

https://deploy-preview-668--jakartaee-specifications.netlify.app/specifications/xml-binding/4.0/ shows the web page preview for the XML Binding 4.0 page update and asks for a Specifications Committee member to promote the TCK.

I created the pull request directly as I think that usually triggers the action more than creating a https://github.com/jakartaee/specifications/issues

@gurunrao
Copy link
Contributor

@gurunrao can you build the tck 4.0.1 from master, please? Thx

As per #83 (comment) feedback from @gurunrao, I should be able to build it via https://ci.eclipse.org/jakartaee-tck/job/10/job/eftl-jaxb-tck-build-run-100 and will do so now to stage the 3.0.1 TCK.

I was away from computers due to holiday on Monday, Thanks @scottmarlow for the release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted Accepted TCK challenge challenge TCK challenge
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants