8176447: javax.xml.validation.Validator validates incorrectly on uniqueness constraint

Reviewed-by: lancea

src/java.xml/share/classes/com/sun/org/apache/xerces/internal/impl/xs/identity/XPathMatcher.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2017, 2019, Oracle and/or its affiliates. All rights reserved.
  */
 /*
  * Licensed to the Apache Software Foundation (ASF) under one or more
@@ -37,6 +37,7 @@ import org.xml.sax.SAXException;
  * @xerces.internal
  *
  * @author Andy Clark, IBM
+ * @LastModified: July 2019
  *
  */
 public class XPathMatcher {
@@ -88,25 +89,25 @@ public class XPathMatcher {
     //
 
     /** XPath location path. */
-    private XPath.LocationPath[] fLocationPaths;
+    private final XPath.LocationPath[] fLocationPaths;
 
     /** True if XPath has been matched. */
-    private int[] fMatched;
+    private final int[] fMatched;
 
     /** The matching string. */
     protected Object fMatchedString;
 
     /** Integer stack of step indexes. */
-    private IntStack[] fStepIndexes;
+    private final IntStack[] fStepIndexes;
 
     /** Current step. */
-    private int[] fCurrentStep;
+    private final int[] fCurrentStep;
 
     /**
      * No match depth. The value of this field will be zero while
      * matching is successful for the given xpath expression.
      */
-    private int [] fNoMatchDepth;
+    private final int [] fNoMatchDepth;
 
     final QName fQName = new QName();
 
@@ -207,7 +208,7 @@ public class XPathMatcher {
      *
      * @throws SAXException Thrown by handler to signal an error.
      */
-    public void startElement(QName element, XMLAttributes attributes){
+    public void startElement(QName element, XMLAttributes attributes) {
         if (DEBUG_METHODS2) {
             System.out.println(toString()+"#startElement("+
                                "element={"+element+"},"+
@@ -215,7 +216,7 @@ public class XPathMatcher {
                                ")");
         }
 
-        for(int i = 0; i < fLocationPaths.length; i++) {
+        for (int i = 0; i < fLocationPaths.length; i++) {
             // push context
             int startStep = fCurrentStep[i];
             fStepIndexes[i].push(startStep);
@@ -284,9 +285,8 @@ public class XPathMatcher {
                 if (DEBUG_MATCH) {
                     System.out.println(toString()+" [CHILD] before");
                 }
-                if (nodeTest.type == XPath.NodeTest.QNAME) {
+                if (!matches(nodeTest, element)) {
-                    if (!nodeTest.name.equals(element)) {
+                    if (fCurrentStep[i] > descendantStep) {
-                        if(fCurrentStep[i] > descendantStep) {
                         fCurrentStep[i] = descendantStep;
                         continue;
                     }
@@ -296,17 +296,17 @@ public class XPathMatcher {
                     }
                     continue;
                 }
-                }
                 fCurrentStep[i]++;
                 if (DEBUG_MATCH) {
                     System.out.println(toString()+" [CHILD] after MATCHED!");
                 }
             }
             if (fCurrentStep[i] == steps.length) {
-                if(sawDescendant) {
+                if (sawDescendant) {
                     fCurrentStep[i] = descendantStep;
                     fMatched[i] = MATCHED_DESCENDANT;
-                } else {
+                }
+                else {
                     fMatched[i] = MATCHED;
                 }
                 continue;
@@ -324,8 +324,7 @@ public class XPathMatcher {
 
                     for (int aIndex = 0; aIndex < attrCount; aIndex++) {
                         attributes.getName(aIndex, fQName);
-                        if (nodeTest.type != XPath.NodeTest.QNAME ||
+                        if (matches(nodeTest, fQName)) {
-                            nodeTest.name.equals(fQName)) {
                             fCurrentStep[i]++;
                             if (fCurrentStep[i] == steps.length) {
                                 fMatched[i] = MATCHED_ATTRIBUTE;
@@ -384,7 +383,7 @@ public class XPathMatcher {
                                "element={"+element+"},"+
                                ")");
         }
-        for(int i = 0; i<fLocationPaths.length; i++) {
+        for (int i = 0; i < fLocationPaths.length; i++) {
             // go back a step
             fCurrentStep[i] = fStepIndexes[i].pop();
 
@@ -395,10 +394,13 @@ public class XPathMatcher {
 
             // signal match, if appropriate
             else {
-                int j=0;
+                int j = 0;
-                for(; j<i && ((fMatched[j] & MATCHED) != MATCHED); j++);
+                for(; j < i && ((fMatched[j] & MATCHED) != MATCHED); j++);
-                if ((j<i) || (fMatched[j] == 0) ||
+                if ((j < i) || (fMatched[j] == 0)) {
-                        ((fMatched[j] & MATCHED_ATTRIBUTE) == MATCHED_ATTRIBUTE)) {
+                    continue;
+                }
+                if ((fMatched[j] & MATCHED_ATTRIBUTE) == MATCHED_ATTRIBUTE) {
+                    fMatched[i] = 0;
                     continue;
                 }
                 // only certain kinds of matchers actually
@@ -476,6 +478,18 @@ public class XPathMatcher {
         return str.toString();
     } // normalize(String):String
 
+    /** Returns true if the given QName matches the node test. **/
+    private static boolean matches(XPath.NodeTest nodeTest, QName value) {
+        if (nodeTest.type == XPath.NodeTest.QNAME) {
+            return nodeTest.name.equals(value);
+        }
+        if (nodeTest.type == XPath.NodeTest.NAMESPACE) {
+            return nodeTest.name.uri == value.uri;
+        }
+        // XPath.NodeTest.WILDCARD
+        return true;
+    } // matches(XPath.NodeTest,QName):boolean
+
     //
     // MAIN
     //

test/jaxp/javax/xml/jaxp/unittest/validation/ValidationTest.java

@@ -36,11 +36,12 @@ import org.testng.annotations.DataProvider;
 
 import org.testng.annotations.Listeners;
 import org.testng.annotations.Test;
+import org.xml.sax.SAXException;
 import org.xml.sax.SAXParseException;
 
 /*
  * @test
- * @bug 8220818
+ * @bug 8220818 8176447
  * @library /javax/xml/jaxp/libs /javax/xml/jaxp/unittest
  * @run testng/othervm validation.ValidationTest
  * @summary Runs validations with schemas and sources
@@ -71,6 +72,17 @@ public class ValidationTest {
         };
     }
 
+    /*
+     DataProvider: uniqueness
+     */
+    @DataProvider(name = "uniqueness")
+    Object[][] getUniqueData() {
+        return new Object[][]{
+            {"JDK8176447a.xsd", "JDK8176447a.xml"},
+            {"JDK8176447b.xsd", "JDK8176447b.xml"},
+        };
+    }
+
     @Test(dataProvider = "invalid", expectedExceptions = SAXParseException.class)
     public void testValidateRefType(String xsd, String xml) throws Exception {
         validate(xsd, xml);
@@ -81,6 +93,19 @@ public class ValidationTest {
         validate(xsd, xml);
     }
 
+    /**
+     * @bug 8176447
+     * Verifies that the uniqueness constraint is checked.
+     * @param xsd the XSD
+     * @param xml the XML
+     * @throws Exception expected when the uniqueness constraint is validated
+     * correctly.
+     */
+    @Test(dataProvider = "uniqueness", expectedExceptions = SAXException.class)
+    public void testUnique(String xsd, String xml) throws Exception {
+        validate(xsd, xml);
+    }
+
     private void validate(String xsd, String xml) throws Exception {
         final SchemaFactory schemaFactory = SchemaFactory.newInstance(
                 XMLConstants.W3C_XML_SCHEMA_NS_URI);
@@ -90,4 +115,5 @@ public class ValidationTest {
         validator.validate(new StreamSource(
                 new File(getClass().getResource(FILE_PATH + xml).getFile())));
     }
+
 }

test/jaxp/javax/xml/jaxp/unittest/validation/files/JDK8176447a.xml

@@ -0,0 +1,8 @@
+<test xmlns="http://openjdk_java_net/test.xml" 
+      xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" 
+      xsi:schemaLocation="JDK8176447a.xsd">
+    <innerObject>
+        <innerInnerObject test-unique-attribute="1" />
+        <innerInnerObject test-unique-attribute="1" />
+    </innerObject>
+</test>

test/jaxp/javax/xml/jaxp/unittest/validation/files/JDK8176447a.xsd

@@ -0,0 +1,24 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<xsd:schema xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns:h="http://www.w3.org/1999/xhtml"
+            xmlns:sn="http://openjdk_java_net/test.xml"
+            targetNamespace="http://openjdk_java_net/test.xml" elementFormDefault="qualified">
+    <xsd:element name="test" type="sn:object">
+        <xsd:unique name="testunique">
+            <xsd:selector xpath="sn:innerObject"/>
+            <xsd:field xpath="sn:innerInnerObject/@test-unique-attribute"/>
+        </xsd:unique>
+    </xsd:element>
+    <xsd:complexType name="object">
+        <xsd:sequence>
+            <xsd:element name="innerObject" maxOccurs="unbounded" type="sn:testType" />
+        </xsd:sequence>
+    </xsd:complexType>
+    <xsd:complexType name="testType">
+        <xsd:sequence>
+            <xsd:element name="innerInnerObject" maxOccurs="unbounded" type="sn:testObjectType"/>
+        </xsd:sequence>
+    </xsd:complexType>
+    <xsd:complexType name="testObjectType">
+        <xsd:attribute use="optional" name="test-unique-attribute" type="xsd:int" />
+    </xsd:complexType>
+</xsd:schema>

test/jaxp/javax/xml/jaxp/unittest/validation/files/JDK8176447b.xml

@@ -0,0 +1,14 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<root xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:noNamespaceSchemaLocation="JDK8176447b.xsd">
+    <e>
+        <e1 a1="a" >
+            <e2 a2="a"/>
+            <e2 a2="a"/>
+        </e1>
+        <e1 a1="a">
+            <e2 a2="b"/>
+            <e2 a2="a"/>
+        </e1>    
+    </e>
+</root>

test/jaxp/javax/xml/jaxp/unittest/validation/files/JDK8176447b.xsd

@@ -0,0 +1,34 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<xs:schema xmlns:xs="http://www.w3.org/2001/XMLSchema" elementFormDefault="qualified">
+  <xs:element name="root">
+    <xs:complexType>
+      <xs:sequence>
+        <xs:element name="e" maxOccurs="unbounded">
+          <xs:complexType>
+            <xs:sequence>
+              <xs:element maxOccurs="unbounded" name="e1">
+                <xs:complexType>
+                  <xs:sequence>
+                    <xs:element maxOccurs="unbounded" name="e2">
+                      <xs:complexType>
+                        <xs:attribute name="a2" use="required" type="xs:NCName"/>
+                      </xs:complexType>
+                    </xs:element>
+                  </xs:sequence>
+                  <xs:attribute name="a1" use="required" type="xs:NCName"/>
+                </xs:complexType>
+              </xs:element>
+            </xs:sequence>
+          </xs:complexType>
+          
+          <xs:key name="checkAttrib">
+            <xs:selector xpath=".//e1"/>
+            <xs:field xpath="@a1"/>
+            <xs:field xpath="e2/@a2"/>
+          </xs:key>
+          
+        </xs:element>
+      </xs:sequence>
+    </xs:complexType>
+  </xs:element>
+</xs:schema>