[bugfix] Compressing persistent Documents or Elements that have non-element Child Nodes, previously caused silent corruption of those documents in the generated archive. This is now fixed so that all nodes are correctly compressed.

adamretter · adamretter · commit 82ced9d36b2d · 2025-06-18T13:06:55.000+02:00
diff --git a/extensions/modules/compression/pom.xml b/extensions/modules/compression/pom.xml
@@ -176,6 +176,7 @@
                                 <exclude>pom.xml</exclude>
                                 <exclude>src/test/resources-filtered/conf.xml</exclude>
                                 <exclude>src/test/resources/log4j2.xml</exclude>
+                                <exclude>src/test/xquery/modules/compression/zip-unzip-tests.xqm</exclude>
                                 <exclude>src/main/java/org/exist/xquery/modules/compression/AbstractCompressFunction.java</exclude>
                                 <exclude>src/main/java/org/exist/xquery/modules/compression/EntryFunctions.java</exclude>
                             </excludes>
diff --git a/extensions/modules/compression/src/main/java/org/exist/xquery/modules/compression/AbstractCompressFunction.java b/extensions/modules/compression/src/main/java/org/exist/xquery/modules/compression/AbstractCompressFunction.java
@@ -66,6 +66,7 @@
 import org.exist.xquery.value.*;
 import org.w3c.dom.Element;
 import org.w3c.dom.Node;
+import org.w3c.dom.NodeList;
 import org.xml.sax.SAXException;
 
 import javax.annotation.Nullable;
@@ -311,13 +312,7 @@ private void compressElement(final OutputStream os, final Element element, final
             throw new XPathException(this, "Item must be type of xs:anyURI or element entry.");
         }
 
-        if (element.getChildNodes().getLength() > 1) {
-            throw new XPathException(this, "Entry content is not valid XML fragment.");
-        }
-
         String name = element.getAttribute("name");
-//            if(name == null)
-//                throw new XPathException(this, "Entry must have name attribute.");
 
         final String type = element.getAttribute("type");
         ZipMethod method;
@@ -328,7 +323,7 @@ private void compressElement(final OutputStream os, final Element element, final
         }
 
         if ("uri".equals(type)) {
-            @Nullable final String uri = element.getFirstChild().getNodeValue();
+            @Nullable final String uri = getChildTextValues(element);
             if (isNullOrEmpty(uri)) {
                 throw new XPathException(this, "Entry with type uri must contain a URI.");
             }
@@ -351,32 +346,57 @@ private void compressElement(final OutputStream os, final Element element, final
             entry = newEntry(name);
 
             if (!"collection".equals(type)) {
-                final byte[] value;
-                final Node content = element.getFirstChild();
+                final NodeList children = element.getChildNodes();
 
-                if (content == null) {
+                final byte[] value;
+                if (children.getLength() == 0) {
                     value = new byte[0];
                 } else {
-                    if (content.getNodeType() == Node.TEXT_NODE) {
-                        String text = content.getNodeValue();
-                        if ("binary".equals(type)) {
-                            //base64 binary
-                            value = Base64.decodeBase64(text);
-                        } else {
-                            //text
-                            value = text.getBytes();
+                    @Nullable Serializer serializer = null;
+                    try (final UnsynchronizedByteArrayOutputStream baos = new UnsynchronizedByteArrayOutputStream()) {
+                        int elementCount = 0;
+                        for (int i = 0; i < children.getLength(); i++) {
+                            final Node child = children.item(i);
+
+                            if (child.getNodeType() == Node.TEXT_NODE) {
+                                final String text = child.getNodeValue();
+                                if ("binary".equals(type)) {
+                                    // Base64 encoded binary
+                                    baos.write(Base64.decodeBase64(text));
+                                } else {
+                                    // Text
+                                    baos.write(text.getBytes());
+                                }
+                            } else {
+                                if (child.getNodeType() == Node.ELEMENT_NODE) {
+                                    elementCount++;
+                                    if (elementCount > 1) {
+                                        throw new XPathException(this, "More than one Element is not permitted within an Entry.");
+                                    }
+                                }
+
+                                // XML
+                                if (serializer == null) {
+                                    serializer = context.getBroker().borrowSerializer();
+                                    serializer.setUser(context.getSubject());
+                                    serializer.setProperty("omit-xml-declaration", "no");
+                                    getDynamicSerializerOptions(serializer);
+                                } else {
+                                    serializer.reset();
+                                    serializer.setUser(context.getSubject());
+                                    getDynamicSerializerOptions(serializer);
+                                    serializer.setProperty("omit-xml-declaration", "yes");
+                                }
+
+                                sbWriter.getBuilder().setLength(0);
+                                serializer.serialize((NodeValue) child, sbWriter);
+                                baos.write(sbWriter.toString().getBytes(StandardCharsets.UTF_8));
+                            }
                         }
-                    } else {
-                        //xml
-                        final Serializer serializer = context.getBroker().borrowSerializer();
-                        try {
-                            serializer.setUser(context.getSubject());
-                            serializer.setProperty("omit-xml-declaration", "no");
-                            getDynamicSerializerOptions(serializer);
-                            sbWriter.getBuilder().setLength(0);
-                            serializer.serialize((NodeValue) content, sbWriter);
-                            value = sbWriter.toString().getBytes(StandardCharsets.UTF_8);
-                        } finally {
+
+                        value = baos.toByteArray();
+                    } finally {
+                        if (serializer != null) {
                             context.getBroker().returnSerializer(serializer);
                         }
                     }
@@ -406,6 +426,36 @@ private void compressElement(final OutputStream os, final Element element, final
         }
 	}
 
+    /**
+     * Get the node values of direct children of the Element
+     * that are Text nodes.
+     *
+     * @param element the element to get child text values from.
+     *
+     * @return the value of the direct child Text nodes, or null if there are no such children.
+     */
+    private @Nullable String getChildTextValues(final Element element) {
+        final NodeList children = element.getChildNodes();
+        final int childLen = children.getLength();
+
+        StringBuilder builder = null;
+        for (int i = 0; i < childLen; i++) {
+            final Node child = children.item(i);
+            if (child.getNodeType() == Node.TEXT_NODE) {
+                if (builder == null) {
+                    builder = new StringBuilder();
+                }
+                builder.append(child.getNodeValue());
+            }
+        }
+
+        if (builder != null) {
+            return builder.toString();
+        }
+
+        return null;
+    }
+
     private void getDynamicSerializerOptions(final Serializer serializer) throws SAXException {
         final Option option = context.getOption(Option.SERIALIZE_QNAME);
         if (option != null) {
diff --git a/extensions/modules/compression/src/test/xquery/modules/compression/zip-unzip-tests.xqm b/extensions/modules/compression/src/test/xquery/modules/compression/zip-unzip-tests.xqm
@@ -0,0 +1,83 @@
+(:
+ : Elemental
+ : Copyright (C) 2024, Evolved Binary Ltd
+ :
+ : admin@evolvedbinary.com
+ : https://www.evolvedbinary.com | https://www.elemental.xyz
+ :
+ : 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; version 2.1.
+ :
+ : 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
+ :)
+xquery version "3.0";
+
+module namespace zut = "http://exist-db.org/xquery/cache/test/zip-unzip";
+
+import module namespace compression = "http://exist-db.org/xquery/compression";
+
+declare namespace test = "http://exist-db.org/xquery/xqsuite";
+declare namespace util = "http://exist-db.org/xquery/util";
+declare namespace xmldb = "http://exist-db.org/xquery/xmldb";
+
+declare variable $zut:TEST_COLLECTION_NAME := "zip-unzip-test";
+declare variable $zut:TEST_COLLECTION_PATH := "/db/" || $zut:TEST_COLLECTION_NAME;
+
+declare variable $zut:DOC_WITH_PIS_1_NAME := "doc-with-pis-1.xml";
+declare variable $zut:DOC_WITH_PIS_1_PATH := $zut:TEST_COLLECTION_PATH || "/" || $zut:DOC_WITH_PIS_1_NAME;
+declare variable $zut:DOC_WITH_PIS_1 := document {
+    <?xml-model href="http://docbook.org/xml/5.0/rng/docbook.rng" schematypens="http://relaxng.org/ns/structure/1.0"?>,
+    <?xml-model href="http://docbook.org/xml/5.0/rng/docbook.rng" type="application/xml" schematypens="http://purl.oclc.org/dsdl/schematron"?>,
+    <article xmlns="http://docbook.org/ns/docbook" xmlns:xlink="http://www.w3.org/1999/xlink" version="5.0">
+      <info>
+        <title>Integration Testing</title>
+        <date>2Q19</date>
+        <keywordset>
+          <keyword>application-development</keyword>
+          <keyword>testing</keyword>
+        </keywordset>
+      </info>
+    </article>
+};
+
+declare
+    %test:setUp
+function zut:setup() {
+    xmldb:create-collection("/db", $zut:TEST_COLLECTION_NAME),
+    xmldb:store($zut:TEST_COLLECTION_PATH, $zut:DOC_WITH_PIS_1_NAME, $zut:DOC_WITH_PIS_1)
+};
+
+declare
+	%test:assertEquals(3)
+function zut:entry-of-doc-with-pis() {
+    let $entries := <entry type="xml" name="{$zut:DOC_WITH_PIS_1_NAME}">{doc($zut:DOC_WITH_PIS_1_PATH)}</entry>
+    return
+        let $zip-data := compression:zip($entries, true())
+        return
+            let $output-collection-path := $zut:TEST_COLLECTION_PATH || "/entry-of-doc-with-pis-output"
+            let $_ := compression:unzip($zip-data, compression:no-filter#2, compression:db-store-entry3($output-collection-path))
+            return
+                count(doc($output-collection-path || "/" || $zut:DOC_WITH_PIS_1_NAME)/node())
+};
+
+declare
+	%test:assertEquals(3)
+function zut:entry-of-doc-child-nodes-including-pis() {
+    let $entries := <entry type="xml" name="{$zut:DOC_WITH_PIS_1_NAME}">{doc($zut:DOC_WITH_PIS_1_PATH)/node()}</entry>
+    return
+        let $zip-data := compression:zip($entries, true())
+        return
+            let $output-collection-path := $zut:TEST_COLLECTION_PATH || "/entry-of-doc-child-nodes-including-pis"
+            let $_ := compression:unzip($zip-data, compression:no-filter#2, compression:db-store-entry3($output-collection-path))
+            return
+                count(doc($output-collection-path || "/" || $zut:DOC_WITH_PIS_1_NAME)/node())
+};
+
