[bugfix] Preserve attribute namespace when copied via enclosed expression (+4 XQTS HEAD)

exist-core/src/main/java/org/exist/dom/memtree/DocumentBuilderReceiver.java

@@ -107,6 +107,7 @@ public XQueryContext getContext() {
 
     @Override
     public void setDocumentLocator(Locator locator) {
+        // no-op: in-memory builder does not surface source-location information.
     }
 
     @Override
@@ -206,12 +207,127 @@ public void characters(final char[] ch, final int start, final int len) throws S
     @Override
     public void attribute(final QName qname, final String value) throws SAXException {
         try {
-            builder.addAttribute(checkNS(false, qname), value);
+            // Attribute namespace handling must be independent of the checkNS
+            // flag: when an attribute is copied into a new element constructor
+            // (XQuery 3.1 §3.9.1.3, default copy-namespaces preserve), the
+            // attribute's prefix MUST be rebound if it conflicts with an
+            // in-scope binding on the new element, and its (prefix, URI)
+            // mapping MUST be reflected as a namespace node on that element.
+            final QName resolved = qname.hasNamespace()
+                    ? resolveAttributeQName(qname)
+                    : qname;
+            builder.addAttribute(resolved, value);
         } catch(final DOMException e) {
             throw new SAXException(e.getMessage(), e);
         }
     }
 
+    /**
+     * Resolves a namespaced attribute QName against the current in-scope
+     * namespaces, rebinding to a freshly generated prefix on prefix-to-URI
+     * conflict, and emitting a namespace node on the parent element to make
+     * the binding visible to the serializer.
+     */
+    private QName resolveAttributeQName(final QName qname) {
+        final XQueryContext context = builder.getContext();
+        final String uri = qname.getNamespaceURI();
+        String prefix = qname.getPrefix();
+        if (prefix == null || prefix.isEmpty()) {
+            // Attribute with namespace but no prefix: pick an existing prefix
+            // mapped to this URI, or generate a fresh one.
+            final String existing = context == null ? null : context.getInScopePrefix(uri);
+            if (existing != null && !existing.isEmpty()) {
+                prefix = existing;
+            } else {
+                prefix = generatePrefix(context, null);
+                if (context != null) {
+                    context.declareInScopeNamespace(prefix, uri);
+                }
+                emitNamespaceNode(prefix, uri);
+                return new QName(qname.getLocalPart(), uri, prefix);
+            }
+        } else if (context != null) {
+            final String boundUri = context.getInScopeNamespace(prefix);
+            if (boundUri == null) {
+                // Prefix is not in scope -> declare it
+                context.declareInScopeNamespace(prefix, uri);
+            } else if (!boundUri.equals(uri)) {
+                // Prefix is bound to a different URI -> generate a fresh prefix
+                String reuse = context.getInScopePrefix(uri);
+                if (reuse == null || reuse.isEmpty()) {
+                    prefix = generatePrefix(context, null);
+                    context.declareInScopeNamespace(prefix, uri);
+                } else {
+                    prefix = reuse;
+                }
+            }
+        }
+        emitNamespaceNode(prefix, uri);
+        return new QName(qname.getLocalPart(), uri, prefix);
+    }
+
+    /**
+     * Adds an xmlns:prefix=uri namespace node to the current element when not
+     * already declared there. No-op for the {@code xml} prefix or when the
+     * parent node is not an element.
+     */
+    private void emitNamespaceNode(final String prefix, final String uri) {
+        if (prefix == null || prefix.isEmpty() || XMLConstants.XML_NS_PREFIX.equals(prefix)) {
+            return;
+        }
+        final DocumentImpl doc = builder.getDocument();
+        final int parent = doc.getLastNode();
+        if (!isElementParent(doc, parent)) {
+            return;
+        }
+        if (isParentSelfDeclaration(doc, parent, prefix, uri)) {
+            return;
+        }
+        if (hasExistingPrefixDeclaration(doc, parent, prefix)) {
+            return;
+        }
+        builder.namespaceNode(prefix, uri);
+    }
+
+    private static boolean isElementParent(final DocumentImpl doc, final int parent) {
+        return parent >= 0 && doc.getNodeType(parent) == org.w3c.dom.Node.ELEMENT_NODE;
+    }
+
+    /**
+     * The parent element already carries the prefix-to-uri binding via its
+     * own name (e.g. parent is {@code <c:foo xmlns:c="..."/>} and we're being
+     * asked to emit {@code xmlns:c="..."} for the same URI). The declaration
+     * is redundant.
+     */
+    private static boolean isParentSelfDeclaration(final DocumentImpl doc, final int parent,
+                                                   final String prefix, final String uri) {
+        final QName parentName = doc.nodeName[parent];
+        return parentName != null
+                && prefix.equals(parentName.getPrefix())
+                && uri.equals(parentName.getNamespaceURI());
+    }
+
+    /**
+     * Scan the namespace declarations already attached to {@code parent} and
+     * return true if any of them binds the same {@code prefix}.
+     */
+    private static boolean hasExistingPrefixDeclaration(final DocumentImpl doc, final int parent,
+                                                       final String prefix) {
+        final int firstNs = doc.alphaLen[parent];
+        if (firstNs < 0) {
+            return false;
+        }
+        for (int ns = firstNs;
+             ns < doc.nextNamespace && doc.namespaceParent[ns] == parent;
+             ns++) {
+            final QName nsName = doc.namespaceCode[ns];
+            if (nsName != null && prefix.equals(nsName.getLocalPart())) {
+                return true;
+            }
+        }
+        return false;
+    }
+
     @Override
     public void ignorableWhitespace(final char[] ch, final int start, final int len) throws SAXException {
         if (!suppressWhitespace) {
@@ -231,18 +347,22 @@ public void cdataSection(final char[] ch, final int start, final int len) throws
 
     @Override
     public void skippedEntity(final String name) throws SAXException {
+        // no-op: entity references are not surfaced through the in-memory builder.
     }
 
     @Override
     public void endCDATA() throws SAXException {
+        // no-op: CDATA boundaries are not surfaced through the in-memory builder.
     }
 
     @Override
     public void endDTD() throws SAXException {
+        // no-op: DTD declarations are not surfaced through the in-memory builder.
     }
 
     @Override
     public void startCDATA() throws SAXException {
+        // no-op: CDATA boundaries are not surfaced through the in-memory builder.
     }
 
     @Override
@@ -257,14 +377,17 @@ public void comment(final char[] ch, final int start, final int length) throws S
 
     @Override
     public void endEntity(final String name) throws SAXException {
+        // no-op: entity boundaries are not surfaced through the in-memory builder.
     }
 
     @Override
     public void startEntity(final String name) throws SAXException {
+        // no-op: entity boundaries are not surfaced through the in-memory builder.
     }
 
     @Override
     public void startDTD(final String name, final String publicId, final String systemId) throws SAXException {
+        // no-op: DTD declarations are not surfaced through the in-memory builder.
     }
 
     @Override
@@ -308,18 +431,18 @@ private QName checkNS(boolean isElement, final QName qname) {
         return qname;
     }
 
-    private String generatePrefix(final XQueryContext context, String prefix) {
+    private String generatePrefix(final XQueryContext context, final String requestedPrefix) {
+        if (requestedPrefix != null) {
+            return requestedPrefix;
+        }
+        // Generate "XXX", "XXX1", "XXX2", ... until we find one not already
+        // bound in scope.
+        String candidate = "XXX";
         int i = 0;
-        while(prefix == null) {
-            prefix = "XXX";
-            if(i > 0) {
-                prefix += String.valueOf(i);
-            }
-            if(context.getInScopeNamespace(prefix) != null) {
-                prefix = null;
-                i++;
-            }
+        while (context.getInScopeNamespace(candidate) != null) {
+            i++;
+            candidate = "XXX" + i;
         }
-        return prefix;
+        return candidate;
     }
 }

exist-core/src/test/java/org/exist/xquery/ElementConstructorAttrNamespaceTest.java

@@ -0,0 +1,102 @@
+/*
+ * eXist-db Open Source Native XML Database
+ * Copyright (C) 2001 The eXist-db Authors
+ *
+ * info@exist-db.org
+ * http://www.exist-db.org
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
+ */
+package org.exist.xquery;
+
+import org.exist.test.ExistXmldbEmbeddedServer;
+import org.junit.ClassRule;
+import org.junit.Test;
+import org.xmldb.api.base.ResourceSet;
+import org.xmldb.api.base.XMLDBException;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+/**
+ * Regression tests for QT4 / XQTS 3.1 prod-DirElemContent.namespace
+ * Constr-inscope-{1..4}: when a namespaced attribute is copied into a
+ * direct element constructor via an enclosed expression, and the prefix
+ * the source attribute uses is already bound to a different namespace URI
+ * on the new constructor, eXist must rebind the attribute to a freshly
+ * generated prefix rather than silently dropping it.
+ *
+ * <p>XQuery 3.1 §3.9.3.4 (Direct Element Constructors) and §4.16
+ * (Copy-Namespaces Declaration). The default copy-namespaces mode is
+ * {@code preserve, inherit}: an attribute's own in-scope namespace MUST
+ * be preserved on the constructed element.</p>
+ */
+public class ElementConstructorAttrNamespaceTest {
+
+    @ClassRule
+    public static final ExistXmldbEmbeddedServer existEmbeddedServer =
+            new ExistXmldbEmbeddedServer(false, true, true);
+
+    /** Constr-inscope-3: prefix on the new constructor conflicts with the copied attribute's prefix. */
+    @Test
+    public void copiedAttributeWithConflictingPrefixIsPreserved() throws XMLDBException {
+        final String xquery = """
+                for $x in <parent1 xmlns:foo="http://www.example.com/parent1" foo:attr1="attr1"/>
+                return <new xmlns:foo="http://www.example.com">{$x//@*:attr1}</new>""";
+        final ResourceSet result = existEmbeddedServer.executeQuery(xquery);
+        assertEquals(1, result.getSize());
+        final String out = result.getResource(0).getContent().toString();
+        // The attribute must survive the copy: its namespace URI is
+        // http://www.example.com/parent1 and its local name is attr1.
+        assertTrue("attribute attr1 must be present in: " + out,
+                out.contains(":attr1=\"attr1\""));
+        assertTrue("source namespace must be declared on the new element: " + out,
+                out.contains("http://www.example.com/parent1"));
+    }
+
+    /** Constr-inscope-4: two attributes with conflicting prefixes copied via enclosed expr. */
+    @Test
+    public void multipleCopiedAttributesWithConflictingPrefixesArePreserved() throws XMLDBException {
+        final String xquery = """
+                for $x in <inscope>
+                            <parent1 xmlns:foo="http://www.example.com/parent1" foo:attr1="attr1"/>
+                            <parent2 xmlns:foo="http://www.example.com/parent2" foo:attr2="attr2"/>
+                          </inscope>
+                return <new>{$x//@*:attr1, $x//@*:attr2}</new>""";
+        final ResourceSet result = existEmbeddedServer.executeQuery(xquery);
+        assertEquals(1, result.getSize());
+        final String out = result.getResource(0).getContent().toString();
+        assertTrue("attr1 must be present: " + out, out.contains(":attr1=\"attr1\""));
+        assertTrue("attr2 must be present: " + out, out.contains(":attr2=\"attr2\""));
+        assertTrue("parent1 namespace must be declared: " + out,
+                out.contains("http://www.example.com/parent1"));
+        assertTrue("parent2 namespace must be declared: " + out,
+                out.contains("http://www.example.com/parent2"));
+    }
+
+    /** Simpler reproducer: rename in-scope namespace using a single copied attribute. */
+    @Test
+    public void copiedAttributeNamespaceRebindsPrefix() throws XMLDBException {
+        final String xquery = """
+                let $src := <s xmlns:foo="http://example.com/A" foo:k="v"/>
+                return <out xmlns:foo="http://example.com/B">{$src/@*}</out>""";
+        final ResourceSet result = existEmbeddedServer.executeQuery(xquery);
+        assertEquals(1, result.getSize());
+        final String out = result.getResource(0).getContent().toString();
+        assertTrue("attribute k must be present: " + out, out.contains(":k=\"v\""));
+        assertTrue("source URI A must be retained on the constructed element: " + out,
+                out.contains("http://example.com/A"));
+    }
+}

exist-core/src/test/java/org/exist/xquery/XQueryTest.java

@@ -1759,7 +1759,14 @@ public void attributeNamespace() throws XMLDBException {
                 XPathQueryService.class);
         ResourceSet result = service.query(query);
         assertEquals(1, result.getSize());
-        assertEquals("<c:C xmlns:c=\"http://c\" xmlns:d=\"http://d\" d:d=\"ddd\">" + "ccc" + "</c:C>", result.getResource(0).getContent().toString());
+        // XQuery 3.1 §2: "the relative order of namespace nodes that share a parent is also implementation dependent."
+        final Source expected = Input.fromString("<c:C xmlns:c=\"http://c\" xmlns:d=\"http://d\" d:d=\"ddd\">ccc</c:C>").build();
+        final Source actual = Input.fromString(result.getResource(0).getContent().toString()).build();
+        final Diff diff = DiffBuilder.compare(expected)
+                .withTest(actual)
+                .checkForIdentical()
+                .build();
+        assertFalse(diff.toString(), diff.hasDifferences());
     }
 
     @Test
@@ -2969,8 +2976,14 @@ public void noNamepaceDefinedForPrefix_1959010() throws XMLDBException {
         ResourceSet result = service.query(query);
 
         assertEquals(1, result.getSize());
-        assertEquals(query, "<c:C xmlns:c=\"http://c\" xmlns:d=\"http://d\" d:d=\"ddd\">ccc</c:C>",
-                result.getResource(0).getContent().toString());
+        // XQuery 3.1 §2: "the relative order of namespace nodes that share a parent is also implementation dependent."
+        final Source expected = Input.fromString("<c:C xmlns:c=\"http://c\" xmlns:d=\"http://d\" d:d=\"ddd\">ccc</c:C>").build();
+        final Source actual = Input.fromString(result.getResource(0).getContent().toString()).build();
+        final Diff diff = DiffBuilder.compare(expected)
+                .withTest(actual)
+                .checkForIdentical()
+                .build();
+        assertFalse(query + "\n" + diff.toString(), diff.hasDifferences());
     }
 
     /**