This is an automated email from the ASF dual-hosted git repository.

rmaucher pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/10.1.x by this push:
     new 1456023c9a Add utility class to restore layout in the new XML file
1456023c9a is described below

commit 1456023c9acbbb11ab8c43d50a993604f5d154fa
Author: remm <[email protected]>
AuthorDate: Thu Sep 17 00:09:55 2026 +0200

    Add utility class to restore layout in the new XML file
    
    Restore comments and layout into the generated XML.
    Although StoreConfig was (hopefully !) functionally correct, dropping
    layout from XML was likely detrimental to actual use of the component.
    With a "large" server, comments and layout become significant.
    At the same time, writing the code for a utility that would preserve
    layout is non trivial (it turned out DOM, which was my immediate idea,
    would work a bit worse than SAX - due to non preservation of attributes
    ordering), and since it is a "cosmetic" item inside a relatively niche
    feature, it is clearly not something that would happen.
    This is a best effort layout restoration process. At the end, the
    functional content is compared, if it does not match for any reason, the
    new raw XML is returned.
    Co authored using OpenCode.
---
 .../catalina/storeconfig/LocalStrings.properties   |   4 +
 .../catalina/storeconfig/StandardContextSF.java    |  18 +-
 .../apache/catalina/storeconfig/StoreConfig.java   |   8 +-
 .../catalina/storeconfig/XMLFormatPreserver.java   | 793 +++++++++++++++++++++
 .../catalina/storeconfig/TestStoreConfig.java      |  71 ++
 .../storeconfig/TestXMLFormatPreserver.java        | 340 +++++++++
 webapps/docs/changelog.xml                         |   6 +
 webapps/docs/config/listeners.xml                  |   8 +
 8 files changed, 1243 insertions(+), 5 deletions(-)

diff --git a/java/org/apache/catalina/storeconfig/LocalStrings.properties 
b/java/org/apache/catalina/storeconfig/LocalStrings.properties
index 874e4de2b7..280faefe5d 100644
--- a/java/org/apache/catalina/storeconfig/LocalStrings.properties
+++ b/java/org/apache/catalina/storeconfig/LocalStrings.properties
@@ -52,3 +52,7 @@ storeFileMover.directoryCreationError=Cannot create directory 
[{0}]
 storeFileMover.null=Invalid null basename, filename or encoding
 storeFileMover.renameError=Cannot rename [{0}] to [{1}]
 storeFileMover.restoreError=Rename [{0}] to [{1}] failed and restoring also 
failed
+
+xmlFormatPreserver.failed=Failed to preserve the original layout of the 
configuration file
+xmlFormatPreserver.unreadable=Failed to read the previous version of the 
configuration file [{0}]
+xmlFormatPreserver.verifyFailed=Failed to verify the configuration file with 
the restored layout, so the layout will not be preserved
diff --git a/java/org/apache/catalina/storeconfig/StandardContextSF.java 
b/java/org/apache/catalina/storeconfig/StandardContextSF.java
index 9afff7cc4f..da2c4445db 100644
--- a/java/org/apache/catalina/storeconfig/StandardContextSF.java
+++ b/java/org/apache/catalina/storeconfig/StandardContextSF.java
@@ -21,6 +21,7 @@ import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.OutputStreamWriter;
 import java.io.PrintWriter;
+import java.io.StringWriter;
 import java.net.URISyntaxException;
 import java.net.URL;
 import java.util.ArrayList;
@@ -141,8 +142,12 @@ public class StandardContextSF extends StoreFactoryBase {
             }
             try (FileOutputStream fos = new FileOutputStream(config);
                     PrintWriter writer = new PrintWriter(new 
OutputStreamWriter(fos, getRegistry().getEncoding()))) {
-                storeXMLHead(writer);
-                super.store(writer, -2, aContext);
+                // Generate the configuration in memory so that the layout of 
the previous version of the file can
+                // be preserved
+                StringWriter buffer = new StringWriter();
+                storeXMLHead(new PrintWriter(buffer));
+                super.store(new PrintWriter(buffer), -2, aContext);
+                writer.write(XMLFormatPreserver.preserve(config, 
buffer.toString(), getRegistry().getEncoding()));
             }
         } else {
             super.store(aWriter, indent, aContext);
@@ -173,8 +178,13 @@ public class StandardContextSF extends StoreFactoryBase {
                         mover.getConfigSave()));
             }
             try (PrintWriter writer = mover.getWriter()) {
-                storeXMLHead(writer);
-                super.store(writer, -2, aContext);
+                // Generate the configuration in memory so that the layout of 
the previous version of the file can
+                // be preserved
+                StringWriter buffer = new StringWriter();
+                storeXMLHead(new PrintWriter(buffer));
+                super.store(new PrintWriter(buffer), -2, aContext);
+                writer.write(XMLFormatPreserver.preserve(mover.getConfigOld(), 
buffer.toString(),
+                        getRegistry().getEncoding()));
             }
             mover.move();
         }
diff --git a/java/org/apache/catalina/storeconfig/StoreConfig.java 
b/java/org/apache/catalina/storeconfig/StoreConfig.java
index 0c4d2ce044..bb861281db 100644
--- a/java/org/apache/catalina/storeconfig/StoreConfig.java
+++ b/java/org/apache/catalina/storeconfig/StoreConfig.java
@@ -17,6 +17,7 @@
 package org.apache.catalina.storeconfig;
 
 import java.io.PrintWriter;
+import java.io.StringWriter;
 import java.net.URL;
 
 import javax.management.MBeanServer;
@@ -203,7 +204,12 @@ public class StoreConfig implements IStoreConfig {
         // Open an output writer for the new configuration file
         try {
             try (PrintWriter writer = mover.getWriter()) {
-                store(writer, -2, aServer);
+                // Generate the configuration in memory so that the layout of 
the previous version of the file can
+                // be preserved
+                StringWriter buffer = new StringWriter();
+                store(new PrintWriter(buffer), -2, aServer);
+                writer.write(XMLFormatPreserver.preserve(mover.getConfigOld(), 
buffer.toString(),
+                        getRegistry().getEncoding()));
             }
             mover.move();
             return true;
diff --git a/java/org/apache/catalina/storeconfig/XMLFormatPreserver.java 
b/java/org/apache/catalina/storeconfig/XMLFormatPreserver.java
new file mode 100644
index 0000000000..041bbb4bd9
--- /dev/null
+++ b/java/org/apache/catalina/storeconfig/XMLFormatPreserver.java
@@ -0,0 +1,793 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.catalina.storeconfig;
+
+import java.io.File;
+import java.io.IOException;
+import java.io.StringReader;
+import java.nio.charset.Charset;
+import java.nio.file.Files;
+import java.util.ArrayDeque;
+import java.util.ArrayList;
+import java.util.Deque;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+
+import javax.xml.parsers.ParserConfigurationException;
+import javax.xml.parsers.SAXParserFactory;
+
+import org.apache.juli.logging.Log;
+import org.apache.juli.logging.LogFactory;
+import org.apache.tomcat.util.res.StringManager;
+import org.apache.tomcat.util.security.Escape;
+import org.xml.sax.Attributes;
+import org.xml.sax.ContentHandler;
+import org.xml.sax.DTDHandler;
+import org.xml.sax.ErrorHandler;
+import org.xml.sax.InputSource;
+import org.xml.sax.Locator;
+import org.xml.sax.SAXException;
+import org.xml.sax.SAXParseException;
+import org.xml.sax.XMLReader;
+import org.xml.sax.ext.LexicalHandler;
+
+/**
+ * Best-effort preservation of the original layout (comments, blank lines and 
attribute order) of <code>server.xml</code>
+ * and context configuration files when StoreConfig rewrites them.
+ * <p>
+ * The freshly generated XML is always the source of truth for the content. 
Both the previous version and the freshly
+ * generated version are parsed, the elements of both documents are matched 
and the freshly generated document is
+ * re-serialized using the layout of the previous version.
+ * <p>
+ * The preservation is best-effort. Whenever the previous version is missing, 
cannot be parsed, or the result cannot
+ * be verified, the freshly generated XML is returned unchanged.
+ */
+public final class XMLFormatPreserver {
+
+    private static final Log log = LogFactory.getLog(XMLFormatPreserver.class);
+
+    /**
+     * The string manager for this package.
+     */
+    private static final StringManager sm = 
StringManager.getManager(Constants.Package);
+
+    /**
+     * The attribute names that strongly identify an element when matching 
elements between the previous and the
+     * freshly generated document.
+     */
+    private static final String[] KEY_ATTRIBUTES = { "className", "name", 
"port", "path", "docBase" };
+
+    private XMLFormatPreserver() {
+        // Utility class, do not instantiate
+    }
+
+    /**
+     * Preserves the layout of the previous version of a configuration file 
for the freshly generated XML.
+     *
+     * @param originalFile The previous version of the configuration file, may 
be {@code null}
+     * @param newXml       The freshly generated XML
+     * @param encoding     The character encoding of the configuration file
+     *
+     * @return The freshly generated XML, re-laid-out using the previous 
version when possible, otherwise the freshly
+     *         generated XML unchanged
+     */
+    public static String preserve(File originalFile, String newXml, String 
encoding) {
+        if (newXml == null || originalFile == null || !originalFile.isFile()) {
+            return newXml;
+        }
+        try {
+            String originalXml = new 
String(Files.readAllBytes(originalFile.toPath()), Charset.forName(encoding));
+            if (originalXml.startsWith("\uFEFF")) {
+                originalXml = originalXml.substring(1);
+            }
+            return preserve(originalXml, newXml, encoding);
+        } catch (Exception e) {
+            log.debug(sm.getString("xmlFormatPreserver.unreadable", 
originalFile), e);
+            return newXml;
+        }
+    }
+
+    /**
+     * Preserves the layout of the previous version of an XML document for the 
freshly generated XML.
+     *
+     * @param originalXml The previous version of the XML document, may be 
{@code null}
+     * @param newXml      The freshly generated XML
+     * @param encoding    The character encoding used in the XML declaration 
of the result
+     *
+     * @return The freshly generated XML, re-laid-out using the previous 
version when possible, otherwise the freshly
+     *         generated XML unchanged
+     */
+    public static String preserve(String originalXml, String newXml, String 
encoding) {
+        if (newXml == null) {
+            return null;
+        }
+        if (originalXml == null || originalXml.isEmpty()) {
+            return newXml;
+        }
+        try {
+            return preserveInternal(originalXml, newXml, (encoding == null || 
encoding.isEmpty()) ? "UTF-8" : encoding);
+        } catch (Exception e) {
+            log.debug(sm.getString("xmlFormatPreserver.failed"), e);
+            return newXml;
+        }
+    }
+
+    /**
+     * Parse both documents, match the elements and re-serialize the freshly 
generated document using the layout of
+     * the previous version.
+     *
+     * @param originalXml The previous version of the XML document
+     * @param newXml      The freshly generated XML
+     * @param encoding    The character encoding used in the XML declaration 
of the result
+     *
+     * @return The re-laid-out XML document
+     *
+     * @throws Exception If one of the documents cannot be parsed or the 
result cannot be verified
+     */
+    private static String preserveInternal(String originalXml, String newXml, 
String encoding) throws Exception {
+        Model original = parse(originalXml);
+        Model fresh = parse(newXml);
+        if (original.root == null || fresh.root == null || 
!original.root.name.equals(fresh.root.name)) {
+            // Not enough overlap to preserve the layout
+            return newXml;
+        }
+        String lineSeparator = originalXml.contains("\r\n") ? "\r\n" : "\n";
+        matchChildren(original.root, fresh.root);
+        StringBuilder result = new StringBuilder(newXml.length() + 512);
+        result.append("<?xml version=\"1.0\" 
encoding=\"").append(encoding).append("\"?>").append(lineSeparator);
+        if (original.doctype != null) {
+            result.append(original.doctype).append(lineSeparator);
+        }
+        emitTokens(result, original.root.preamble, 0, lineSeparator);
+        emitElement(result, fresh.root, 0, lineSeparator);
+        emitTokens(result, original.trailing, 0, lineSeparator);
+        String formatted = result.toString();
+        // Verify that the re-laid-out document still carries exactly the same 
content as the freshly generated one
+        Model check = parse(formatted);
+        if (check.root == null || !contentEquals(fresh.root, check.root)) {
+            log.debug(sm.getString("xmlFormatPreserver.verifyFailed"));
+            return newXml;
+        }
+        return formatted;
+    }
+
+    /**
+     * Parse an XML document into a model that also captures the layout 
information (comments, blank lines, attribute
+     * order).
+     *
+     * @param xml The XML document to parse
+     *
+     * @return The parsed model
+     *
+     * @throws SAXException                 If the document is not well-formed
+     * @throws IOException                  If the document cannot be read
+     * @throws ParserConfigurationException If the parser cannot be configured
+     */
+    private static Model parse(String xml) throws SAXException, IOException, 
ParserConfigurationException {
+        SAXParserFactory factory = SAXParserFactory.newInstance();
+        factory.setNamespaceAware(false);
+        factory.setValidating(false);
+        
factory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd";,
 false);
+        disableFeature(factory, 
"http://xml.org/sax/features/external-general-entities";);
+        disableFeature(factory, 
"http://xml.org/sax/features/external-parameter-entities";);
+        Model model = new Model();
+        ModelBuilder builder = new ModelBuilder(model);
+        XMLReader reader = factory.newSAXParser().getXMLReader();
+        reader.setContentHandler(builder);
+        reader.setDTDHandler(builder);
+        reader.setErrorHandler(builder);
+        reader.setProperty("http://xml.org/sax/properties/lexical-handler";, 
builder);
+        reader.parse(new InputSource(new StringReader(xml)));
+        return model;
+    }
+
+    /**
+     * Disable a parser feature, ignoring parsers that do not support it.
+     *
+     * @param factory  The parser factory
+     * @param feature  The feature to disable
+     */
+    private static void disableFeature(SAXParserFactory factory, String 
feature) {
+        try {
+            factory.setFeature(feature, false);
+        } catch (ParserConfigurationException ignore) {
+            // The parser does not support this feature. Continue with the 
remaining protections.
+        } catch (SAXException ignore) {
+            // The parser does not support this feature. Continue with the 
remaining protections.
+        }
+    }
+
+    /**
+     * Match the element children of the previous version with the element 
children of the freshly generated version.
+     * The match of each freshly generated element is stored in {@link 
XmlElement#match}.
+     *
+     * @param original The element of the previous version
+     * @param fresh    The matching element of the freshly generated version
+     */
+    private static void matchChildren(XmlElement original, XmlElement fresh) {
+        Map<String, List<XmlElement>> candidatesByName = new HashMap<>();
+        for (XmlElement child : original.children) {
+            candidatesByName.computeIfAbsent(child.name, (name) -> new 
ArrayList<>()).add(child);
+        }
+        for (XmlElement freshChild : fresh.children) {
+            List<XmlElement> candidates = 
candidatesByName.get(freshChild.name);
+            if (candidates == null) {
+                continue;
+            }
+            candidates.removeIf((candidate) -> candidate.matched);
+            if (candidates.isEmpty()) {
+                continue;
+            }
+            XmlElement best = null;
+            int bestScore = 0;
+            for (XmlElement candidate : candidates) {
+                int score = matchScore(freshChild, candidate);
+                if (score > bestScore) {
+                    bestScore = score;
+                    best = candidate;
+                }
+            }
+            if (best == null && candidates.size() == 1) {
+                // A unique candidate with the same tag name is assumed to be 
the match
+                best = candidates.get(0);
+            }
+            if (best != null) {
+                best.matched = true;
+                freshChild.match = best;
+                matchChildren(best, freshChild);
+            }
+        }
+    }
+
+    /**
+     * Score how well a candidate element of the previous version matches a 
freshly generated element.
+     *
+     * @param fresh    The freshly generated element
+     * @param candidate The candidate element of the previous version
+     *
+     * @return The match score, 0 if nothing is shared
+     */
+    private static int matchScore(XmlElement fresh, XmlElement candidate) {
+        // Identical attribute sets (the order may differ) are the strongest 
match
+        if (attributesEqual(fresh, candidate)) {
+            return 1000;
+        }
+        int score = 0;
+        for (int i = 0; i < fresh.attributeNames.size(); i++) {
+            String name = fresh.attributeNames.get(i);
+            String value = fresh.attributeValues.get(i);
+            int index = candidate.attributeIndex(name);
+            if (index >= 0 && 
candidate.attributeValues.get(index).equals(value)) {
+                score += isKeyAttribute(name) ? 100 : 10;
+            }
+        }
+        String freshText = fresh.getText();
+        String candidateText = candidate.getText();
+        if (freshText != null && !freshText.trim().isEmpty()
+                && Objects.equals(freshText.trim(), candidateText == null ? 
null : candidateText.trim())) {
+            score += 50;
+        }
+        return score;
+    }
+
+    /**
+     * Check whether two elements have the same attribute set (the order may 
differ).
+     *
+     * @param a The first element
+     * @param b The second element
+     *
+     * @return {@code true} if both elements have the same attribute set
+     */
+    private static boolean attributesEqual(XmlElement a, XmlElement b) {
+        if (a.attributeNames.size() != b.attributeNames.size()) {
+            return false;
+        }
+        for (int i = 0; i < a.attributeNames.size(); i++) {
+            int index = b.attributeIndex(a.attributeNames.get(i));
+            if (index < 0 || 
!a.attributeValues.get(i).equals(b.attributeValues.get(index))) {
+                return false;
+            }
+        }
+        return true;
+    }
+
+    /**
+     * Check whether an attribute name strongly identifies an element.
+     *
+     * @param name The attribute name
+     *
+     * @return {@code true} if the attribute name is a key attribute
+     */
+    private static boolean isKeyAttribute(String name) {
+        for (String key : KEY_ATTRIBUTES) {
+            if (key.equals(name)) {
+                return true;
+            }
+        }
+        return false;
+    }
+
+    /**
+     * Serialize an element of the freshly generated document, decorated with 
the layout of its match in the previous
+     * version.
+     *
+     * @param result        The output buffer
+     * @param fresh         The element of the freshly generated document
+     * @param indent        The indentation in spaces
+     * @param lineSeparator The line separator to use
+     */
+    private static void emitElement(StringBuilder result, XmlElement fresh, 
int indent, String lineSeparator) {
+        XmlElement original = fresh.match;
+        String indentString = spaces(indent);
+        if (original != null) {
+            emitTokens(result, original.preamble, indent, lineSeparator);
+        }
+        boolean hasChildren = !fresh.children.isEmpty();
+        String text = fresh.getText();
+        boolean hasText = text != null && !text.trim().isEmpty();
+        // Reproduce the attribute line wrapping of StoreAppender: the 
position starts at the indentation level and
+        // the attributes are wrapped on a new line once the position exceeds 
60 characters
+        int position = indent;
+        int wrapIndent = hasChildren ? indent + 4 : indent + 2;
+        List<String> orderedNames = new ArrayList<>(fresh.attributeNames);
+        if (original != null) {
+            // Use the attribute order of the previous version. Attributes 
that are only present in the freshly
+            // generated document keep their position at the end
+            List<String> reordered = new 
ArrayList<>(fresh.attributeNames.size());
+            for (String name : original.attributeNames) {
+                if (fresh.attributeIndex(name) >= 0) {
+                    reordered.add(name);
+                }
+            }
+            for (String name : orderedNames) {
+                if (!reordered.contains(name)) {
+                    reordered.add(name);
+                }
+            }
+            orderedNames = reordered;
+        }
+        result.append(indentString).append('<').append(fresh.name);
+        for (String name : orderedNames) {
+            String value = 
Escape.xml(fresh.attributeValues.get(fresh.attributeIndex(name)));
+            position += name.length() + value.length();
+            if (position > 60) {
+                result.append(lineSeparator).append(spaces(wrapIndent));
+                position = wrapIndent;
+            } else {
+                result.append(' ');
+            }
+            result.append(name).append("=\"").append(value).append("\"");
+        }
+        if (!hasChildren && !hasText) {
+            result.append("/>").append(lineSeparator);
+            return;
+        }
+        result.append('>');
+        if (hasText && !hasChildren) {
+            
result.append(Escape.xml(text)).append("</").append(fresh.name).append('>').append(lineSeparator);
+            return;
+        }
+        result.append(lineSeparator);
+        for (XmlElement child : fresh.children) {
+            emitElement(result, child, indent + 2, lineSeparator);
+        }
+        if (original != null) {
+            emitTokens(result, original.beforeClose, indent, lineSeparator);
+        }
+        
result.append(indentString).append("</").append(fresh.name).append('>').append(lineSeparator);
+    }
+
+    /**
+     * Emit the layout tokens (comments and blank lines) of the previous 
version.
+     *
+     * @param result        The output buffer
+     * @param tokens        The layout tokens, may be {@code null}
+     * @param indent        The indentation in spaces
+     * @param lineSeparator The line separator to use
+     */
+    private static void emitTokens(StringBuilder result, List<Token> tokens, 
int indent, String lineSeparator) {
+        if (tokens == null || tokens.isEmpty()) {
+            return;
+        }
+        String indentString = spaces(indent);
+        for (Token token : tokens) {
+            if (token.comment == null) {
+                for (int i = 0; i < token.blankLines; i++) {
+                    result.append(lineSeparator);
+                }
+            } else {
+                // Keep the indentation the comment had in the previous 
version if it is deeper than the current
+                // indentation level
+                String commentIndent = token.indentHint.length() > 
indentString.length() ? token.indentHint
+                        : indentString;
+                emitComment(result, token.comment, commentIndent, 
lineSeparator);
+            }
+        }
+    }
+
+    /**
+     * Emit a comment of the previous version. The first line is aligned to 
the previous indentation level, the
+     * remaining lines keep their original layout.
+     *
+     * @param result        The output buffer
+     * @param comment       The comment text, without the delimiters
+     * @param indentString  The indentation for the first line
+     * @param lineSeparator The line separator to use
+     */
+    private static void emitComment(StringBuilder result, String comment, 
String indentString, String lineSeparator) {
+        String normalized = comment.replace("\r\n", "\n");
+        String[] lines = normalized.split("\n", -1);
+        result.append(indentString).append("<!--").append(lines[0]);
+        if (lines.length == 1) {
+            result.append("-->").append(lineSeparator);
+            return;
+        }
+        result.append(lineSeparator);
+        for (int i = 1; i < lines.length; i++) {
+            if (i < lines.length - 1) {
+                result.append(lines[i]).append(lineSeparator);
+            } else if (lines[i].isEmpty()) {
+                result.append("-->").append(lineSeparator);
+            } else {
+                result.append(lines[i]).append("-->").append(lineSeparator);
+            }
+        }
+    }
+
+    /**
+     * Compare the content (element names, attributes and text) of two 
elements, ignoring layout information.
+     *
+     * @param a The first element, may be {@code null}
+     * @param b The second element, may be {@code null}
+     *
+     * @return {@code true} if both elements carry the same content
+     */
+    private static boolean contentEquals(XmlElement a, XmlElement b) {
+        if (a == null || b == null) {
+            return a == b;
+        }
+        if (!a.name.equals(b.name)) {
+            return false;
+        }
+        if (a.attributeNames.size() != b.attributeNames.size()) {
+            return false;
+        }
+        for (int i = 0; i < a.attributeNames.size(); i++) {
+            int index = b.attributeIndex(a.attributeNames.get(i));
+            if (index < 0 || 
!a.attributeValues.get(i).equals(b.attributeValues.get(index))) {
+                return false;
+            }
+        }
+        // Only the significant text is compared: whitespace between child 
elements is layout, not content
+        String aText = a.getText();
+        String bText = b.getText();
+        String aTrimmed = (aText == null || aText.trim().isEmpty()) ? null : 
aText.trim();
+        String bTrimmed = (bText == null || bText.trim().isEmpty()) ? null : 
bText.trim();
+        if (!Objects.equals(aTrimmed, bTrimmed)) {
+            return false;
+        }
+        if (a.children.size() != b.children.size()) {
+            return false;
+        }
+        for (int i = 0; i < a.children.size(); i++) {
+            if (!contentEquals(a.children.get(i), b.children.get(i))) {
+                return false;
+            }
+        }
+        return true;
+    }
+
+    /**
+     * Create a string of spaces.
+     *
+     * @param count The number of spaces
+     *
+     * @return The string of spaces
+     */
+    private static String spaces(int count) {
+        StringBuilder result = new StringBuilder(count);
+        for (int i = 0; i < count; i++) {
+            result.append(' ');
+        }
+        return result.toString();
+    }
+
+    /**
+     * The parsed representation of an XML document.
+     */
+    private static final class Model {
+        private XmlElement root;
+        private List<Token> trailing = new ArrayList<>();
+        private String doctype;
+    }
+
+    /**
+     * A parsed XML element, including the layout information of the document 
it was parsed from.
+     */
+    private static final class XmlElement {
+        private final String name;
+        private final List<String> attributeNames = new ArrayList<>();
+        private final List<String> attributeValues = new ArrayList<>();
+        private final List<XmlElement> children = new ArrayList<>();
+        private StringBuilder text;
+        // Layout of the document this element was parsed from
+        private List<Token> preamble = new ArrayList<>();
+        private List<Token> beforeClose = new ArrayList<>();
+        private List<Token> pending = new ArrayList<>();
+        // Matching with the other document
+        private XmlElement match;
+        private boolean matched;
+
+        private XmlElement(String name) {
+            this.name = name;
+        }
+
+        private void appendText(String text) {
+            if (this.text == null) {
+                this.text = new StringBuilder();
+            }
+            this.text.append(text);
+        }
+
+        private String getText() {
+            return text == null ? null : text.toString();
+        }
+
+        private int attributeIndex(String name) {
+            for (int i = 0; i < attributeNames.size(); i++) {
+                if (attributeNames.get(i).equals(name)) {
+                    return i;
+                }
+            }
+            return -1;
+        }
+    }
+
+    /**
+     * A layout token: a comment or a run of blank lines.
+     */
+    private static final class Token {
+        private final String comment;
+        private final int blankLines;
+        // The indentation the comment had in the document it was parsed from
+        private final String indentHint;
+
+        private Token(String comment, int blankLines, String indentHint) {
+            this.comment = comment;
+            this.blankLines = blankLines;
+            this.indentHint = indentHint;
+        }
+
+        static Token comment(String comment, String indentHint) {
+            return new Token(comment, 0, indentHint);
+        }
+
+        static Token blankLines(int count) {
+            return new Token(null, count, "");
+        }
+    }
+
+    /**
+     * SAX handler that builds a {@link Model} from an XML document.
+     */
+    private static final class ModelBuilder implements ContentHandler, 
DTDHandler, ErrorHandler, LexicalHandler {
+
+        private final Model model;
+        private final Deque<XmlElement> stack = new ArrayDeque<>();
+        private final StringBuilder gap = new StringBuilder();
+
+        ModelBuilder(Model model) {
+            this.model = model;
+            // The document node is the base of the stack. Its children are 
the document elements.
+            stack.push(new XmlElement(null));
+        }
+
+        /**
+         * If the whitespace accumulated since the last layout token contains 
at least two line breaks, add a blank
+         * lines token to the current pending tokens.
+         */
+        private void flushGap() {
+            XmlElement top = stack.peek();
+            int newlines = 0;
+            for (int i = 0; i < gap.length(); i++) {
+                if (gap.charAt(i) == '\n') {
+                    newlines++;
+                }
+            }
+            gap.setLength(0);
+            if (newlines >= 2) {
+                top.pending.add(Token.blankLines(newlines - 1));
+            }
+        }
+
+        /**
+         * The whitespace after the last line break of the current gap, i.e. 
the indentation a following comment had
+         * in the document.
+         *
+         * @return The indentation hint, an empty string if there is none
+         */
+        private String gapIndentHint() {
+            int lastNewline = gap.lastIndexOf("\n");
+            return lastNewline < 0 ? "" : gap.substring(lastNewline + 1);
+        }
+
+        @Override
+        public void startElement(String uri, String localName, String qName, 
Attributes attributes)
+                throws SAXException {
+            flushGap();
+            XmlElement parent = stack.peek();
+            XmlElement element = new XmlElement(qName);
+            for (int i = 0; i < attributes.getLength(); i++) {
+                // The SAX attribute order is the document order
+                element.attributeNames.add(attributes.getQName(i));
+                element.attributeValues.add(attributes.getValue(i));
+            }
+            // The layout tokens of the parent, accumulated since its last 
child, belong to this element
+            element.preamble = parent.pending;
+            parent.pending = new ArrayList<>();
+            parent.children.add(element);
+            stack.push(element);
+        }
+
+        @Override
+        public void endElement(String uri, String localName, String qName) 
throws SAXException {
+            XmlElement element = stack.pop();
+            flushGap();
+            element.beforeClose = element.pending;
+        }
+
+        @Override
+        public void endDocument() throws SAXException {
+            flushGap();
+            XmlElement document = stack.peek();
+            model.root = document.children.isEmpty() ? null : 
document.children.get(0);
+            model.trailing = document.pending;
+        }
+
+        @Override
+        public void characters(char[] ch, int start, int length) throws 
SAXException {
+            String text = new String(ch, start, length);
+            if (text.isEmpty()) {
+                return;
+            }
+            XmlElement top = stack.peek();
+            if (top.name != null) {
+                top.appendText(text);
+            }
+            boolean whitespace = true;
+            for (int i = 0; i < text.length(); i++) {
+                if (!Character.isWhitespace(text.charAt(i))) {
+                    whitespace = false;
+                    break;
+                }
+            }
+            if (whitespace) {
+                gap.append(text);
+            } else {
+                gap.setLength(0);
+            }
+        }
+
+        @Override
+        public void comment(char[] ch, int start, int length) throws 
SAXException {
+            String indentHint = gapIndentHint();
+            flushGap();
+            stack.peek().pending.add(Token.comment(new String(ch, start, 
length), indentHint));
+        }
+
+        @Override
+        public void startDTD(String name, String publicId, String systemId) 
throws SAXException {
+            StringBuilder doctype = new StringBuilder("<!DOCTYPE 
").append(name);
+            if (publicId != null) {
+                doctype.append(" PUBLIC \"").append(publicId).append("\" \"")
+                        .append(systemId == null ? "" : systemId).append("\"");
+            } else if (systemId != null) {
+                doctype.append(" SYSTEM \"").append(systemId).append("\"");
+            }
+            doctype.append('>');
+            model.doctype = doctype.toString();
+        }
+
+        @Override
+        public void error(SAXParseException exception) throws SAXException {
+            throw exception;
+        }
+
+        @Override
+        public void fatalError(SAXParseException exception) throws 
SAXException {
+            throw exception;
+        }
+
+        @Override
+        public void warning(SAXParseException exception) throws SAXException {
+            // Ignore
+        }
+
+        @Override
+        public void setDocumentLocator(Locator locator) {
+            // Not used
+        }
+
+        @Override
+        public void startDocument() throws SAXException {
+            // Not used
+        }
+
+        @Override
+        public void startPrefixMapping(String prefix, String uri) throws 
SAXException {
+            // Not used
+        }
+
+        @Override
+        public void endPrefixMapping(String prefix) throws SAXException {
+            // Not used
+        }
+
+        @Override
+        public void ignorableWhitespace(char[] ch, int start, int length) 
throws SAXException {
+            characters(ch, start, length);
+        }
+
+        @Override
+        public void processingInstruction(String target, String data) throws 
SAXException {
+            // Not used
+        }
+
+        @Override
+        public void skippedEntity(String name) throws SAXException {
+            // Not used
+        }
+
+        @Override
+        public void endDTD() throws SAXException {
+            // Not used
+        }
+
+        @Override
+        public void startEntity(String name) throws SAXException {
+            // Not used
+        }
+
+        @Override
+        public void endEntity(String name) throws SAXException {
+            // Not used
+        }
+
+        @Override
+        public void startCDATA() throws SAXException {
+            // Not used
+        }
+
+        @Override
+        public void endCDATA() throws SAXException {
+            // Not used
+        }
+
+        @Override
+        public void notationDecl(String name, String publicId, String 
systemId) throws SAXException {
+            // Not used
+        }
+
+        @Override
+        public void unparsedEntityDecl(String name, String publicId, String 
systemId, String notationName)
+                throws SAXException {
+            // Not used
+        }
+    }
+}
diff --git a/test/org/apache/catalina/storeconfig/TestStoreConfig.java 
b/test/org/apache/catalina/storeconfig/TestStoreConfig.java
index 5ece5707c0..072d99d2b3 100644
--- a/test/org/apache/catalina/storeconfig/TestStoreConfig.java
+++ b/test/org/apache/catalina/storeconfig/TestStoreConfig.java
@@ -20,6 +20,8 @@ import java.io.File;
 import java.io.FileReader;
 import java.io.StringReader;
 import java.io.StringWriter;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
 
 import javax.xml.parsers.SAXParserFactory;
 
@@ -27,11 +29,13 @@ import org.junit.Assert;
 import org.junit.Test;
 
 import org.apache.catalina.connector.Connector;
+import org.apache.catalina.realm.LockOutRealm;
 import org.apache.catalina.startup.Catalina;
 import org.apache.catalina.startup.CatalinaBaseConfigurationSource;
 import org.apache.catalina.startup.Tomcat;
 import org.apache.catalina.startup.TomcatBaseTest;
 import org.apache.catalina.util.IOTools;
+import org.apache.catalina.valves.AccessLogValve;
 import org.xml.sax.InputSource;
 
 public class TestStoreConfig extends TomcatBaseTest {
@@ -161,4 +165,71 @@ public class TestStoreConfig extends TomcatBaseTest {
         tomcat.stop();
     }
 
+    /**
+     * Verify that StoreConfig preserves the comments of the existing 
server.xml when it rewrites the file.
+     *
+     * @throws Exception if the test experiences an unexpected error
+     */
+    @Test
+    public void testStorePreservesComments() throws Exception {
+        Tomcat tomcat = getTomcatInstance();
+        StoreConfigLifecycleListener storeConfigListener = new 
StoreConfigLifecycleListener();
+        tomcat.getServer().addLifecycleListener(storeConfigListener);
+
+        // Use a storable realm. The default embedded realm 
(Tomcat.SimpleRealm) is an inner class that the store
+        // path cannot instantiate a default instance of.
+        tomcat.getEngine().setRealm(new LockOutRealm());
+
+        // Add a component so that the stored configuration differs from the 
original file
+        AccessLogValve accessLogValve = new AccessLogValve();
+        accessLogValve.setDirectory("logs");
+        accessLogValve.setPrefix("localhost_access_log");
+        accessLogValve.setSuffix(".txt");
+        accessLogValve.setPattern("%h %l %u %t \"%r\" %s %b");
+        tomcat.getHost().getPipeline().addValve(accessLogValve);
+
+        // Write a server.xml with comments that StoreConfig must preserve
+        File conf = new File(getTemporaryDirectory(), "conf");
+        if (!conf.mkdirs()) {
+            Assert.fail("Unable to create conf directory");
+        }
+        // Delete the whole conf directory (including any timestamped backup) 
after the test
+        addDeleteOnTearDown(conf);
+        File serverXml = new File(conf, "server.xml");
+        Files.write(serverXml.toPath(), String.join("\n",
+                "<?xml version=\"1.0\" encoding=\"UTF-8\"?>",
+                "<!--",
+                "  Header comment line",
+                "-->",
+                "<Server port=\"8005\" shutdown=\"SHUTDOWN\">",
+                "    <!-- Service comment -->",
+                "    <Service name=\"Catalina\">",
+                "        <!-- Commented out connector",
+                "        <Connector port=\"8009\"/>",
+                "        -->",
+                "    </Service>",
+                "</Server>",
+                "").getBytes(StandardCharsets.UTF_8));
+
+        tomcat.start();
+
+        // Save configuration
+        storeConfigListener.getStoreConfig().storeConfig();
+
+        // Read written configuration
+        String serverXmlDump;
+        try (FileReader reader = new FileReader(serverXml);
+                StringWriter writer = new StringWriter()) {
+            IOTools.flow(reader, writer);
+            serverXmlDump = writer.toString();
+        }
+        Assert.assertTrue(serverXmlDump.contains("Header comment line"));
+        Assert.assertTrue(serverXmlDump.contains("Service comment"));
+        Assert.assertTrue(serverXmlDump.contains("Commented out connector"));
+        Assert.assertTrue(serverXmlDump.contains("AccessLogValve"));
+        // The stored configuration must remain well-formed
+        SAXParserFactory.newInstance().newSAXParser().getXMLReader()
+                .parse(new InputSource(new StringReader(serverXmlDump)));
+    }
+
 }
diff --git a/test/org/apache/catalina/storeconfig/TestXMLFormatPreserver.java 
b/test/org/apache/catalina/storeconfig/TestXMLFormatPreserver.java
new file mode 100644
index 0000000000..417845bc12
--- /dev/null
+++ b/test/org/apache/catalina/storeconfig/TestXMLFormatPreserver.java
@@ -0,0 +1,340 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.catalina.storeconfig;
+
+import java.io.File;
+import java.io.StringReader;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+
+import javax.xml.parsers.SAXParserFactory;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import org.xml.sax.InputSource;
+
+/**
+ * Unit tests for {@link XMLFormatPreserver}.
+ */
+public class TestXMLFormatPreserver {
+
+    @Test
+    public void testNullOriginal() {
+        String newXml = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<Server 
port=\"8005\"/>\n";
+        Assert.assertEquals(newXml, XMLFormatPreserver.preserve((File) null, 
newXml, "UTF-8"));
+        Assert.assertEquals(newXml, XMLFormatPreserver.preserve("", newXml, 
"UTF-8"));
+        Assert.assertNull(XMLFormatPreserver.preserve("<Server/>", null, 
"UTF-8"));
+    }
+
+    @Test
+    public void testMalformedDocuments() {
+        String newXml = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<Server 
port=\"8005\"/>\n";
+        Assert.assertEquals(newXml, XMLFormatPreserver.preserve("<broken", 
newXml, "UTF-8"));
+        Assert.assertEquals("<broken", 
XMLFormatPreserver.preserve("<Server/>", "<broken", "UTF-8"));
+    }
+
+    @Test
+    public void testDifferentRootElements() {
+        Assert.assertEquals("<Server/>", 
XMLFormatPreserver.preserve("<Host/>", "<Server/>", "UTF-8"));
+    }
+
+    @Test
+    public void testCommentsAndBlankLinesPreserved() {
+        String originalXml = String.join("\n",
+                "<?xml version=\"1.0\" encoding=\"UTF-8\"?>",
+                "<!--",
+                "  License header line 1",
+                "  License header line 2",
+                "-->",
+                "<Server port=\"8005\">",
+                "    <!-- Section comment -->",
+                "",
+                "    <Service name=\"Catalina\">",
+                "        <Connector port=\"8080\"/>",
+                "        <!-- Commented out element",
+                "        <Connector port=\"8009\"/>",
+                "        -->",
+                "    </Service>",
+                "</Server>",
+                "");
+        String newXml = String.join("\n",
+                "<?xml version=\"1.0\" encoding=\"UTF-8\"?>",
+                "<Server port=\"8006\">",
+                "  <Service name=\"Catalina\">",
+                "    <Connector port=\"8080\"/>",
+                "  </Service>",
+                "</Server>",
+                "");
+        String expected = String.join("\n",
+                "<?xml version=\"1.0\" encoding=\"UTF-8\"?>",
+                "<!--",
+                "  License header line 1",
+                "  License header line 2",
+                "-->",
+                "<Server port=\"8006\">",
+                "    <!-- Section comment -->",
+                "",
+                "  <Service name=\"Catalina\">",
+                "    <Connector port=\"8080\"/>",
+                "        <!-- Commented out element",
+                "        <Connector port=\"8009\"/>",
+                "        -->",
+                "  </Service>",
+                "</Server>",
+                "");
+        Assert.assertEquals(expected, XMLFormatPreserver.preserve(originalXml, 
newXml, "UTF-8"));
+    }
+
+    @Test
+    public void testAttributeOrderPreserved() {
+        String originalXml = "<?xml version=\"1.0\" 
encoding=\"UTF-8\"?>\n<Server>\n"
+                + "  <Connector protocol=\"HTTP/1.1\" 
port=\"8080\"/>\n</Server>\n";
+        String newXml = "<?xml version=\"1.0\" 
encoding=\"UTF-8\"?>\n<Server>\n"
+                + "  <Connector port=\"8080\" protocol=\"HTTP/1.1\" 
maxThreads=\"200\"/>\n</Server>\n";
+        String result = XMLFormatPreserver.preserve(originalXml, newXml, 
"UTF-8");
+        Assert.assertTrue(result, result.contains("<Connector 
protocol=\"HTTP/1.1\" port=\"8080\" maxThreads=\"200\"/>"));
+    }
+
+    @Test
+    public void testAttributeWrapping() {
+        String originalXml = "<?xml version=\"1.0\" 
encoding=\"UTF-8\"?>\n<Server>\n"
+                + "  <Connector 
longAttributeOne=\"123456789012345678901234567890\"\n"
+                + "    longAttributeTwo=\"ABCDEFGHIJ\"/>\n</Server>\n";
+        String newXml = "<?xml version=\"1.0\" 
encoding=\"UTF-8\"?>\n<Server>\n"
+                + "  <Connector 
longAttributeOne=\"123456789012345678901234567890\"\n"
+                + "    longAttributeTwo=\"K\"/>\n</Server>\n";
+        String expected = "<?xml version=\"1.0\" 
encoding=\"UTF-8\"?>\n<Server>\n"
+                + "  <Connector 
longAttributeOne=\"123456789012345678901234567890\"\n"
+                + "    longAttributeTwo=\"K\"/>\n</Server>\n";
+        Assert.assertEquals(expected, XMLFormatPreserver.preserve(originalXml, 
newXml, "UTF-8"));
+    }
+
+    @Test
+    public void testSiblingMatchingByKeyAttributes() {
+        String originalXml = String.join("\n",
+                "<Server>",
+                "    <!-- SSL connector -->",
+                "    <Connector port=\"8443\"/>",
+                "    <!-- HTTP connector -->",
+                "    <Connector port=\"8080\"/>",
+                "</Server>",
+                "");
+        String newXml = String.join("\n",
+                "<Server>",
+                "  <Connector port=\"8081\"/>",
+                "  <Connector port=\"8443\"/>",
+                "</Server>",
+                "");
+        String expected = String.join("\n",
+                "<?xml version=\"1.0\" encoding=\"UTF-8\"?>",
+                "<Server>",
+                "  <Connector port=\"8081\"/>",
+                "    <!-- SSL connector -->",
+                "  <Connector port=\"8443\"/>",
+                "</Server>",
+                "");
+        Assert.assertEquals(expected, XMLFormatPreserver.preserve(originalXml, 
newXml, "UTF-8"));
+    }
+
+    @Test
+    public void testNewElementIsNotDecorated() {
+        String originalXml = String.join("\n",
+                "<Server>",
+                "    <!-- HTTP -->",
+                "    <Connector port=\"8080\"/>",
+                "</Server>",
+                "");
+        String newXml = String.join("\n",
+                "<Server>",
+                "  <Connector port=\"8080\"/>",
+                "  <Connector port=\"8443\"/>",
+                "</Server>",
+                "");
+        String expected = String.join("\n",
+                "<?xml version=\"1.0\" encoding=\"UTF-8\"?>",
+                "<Server>",
+                "    <!-- HTTP -->",
+                "  <Connector port=\"8080\"/>",
+                "  <Connector port=\"8443\"/>",
+                "</Server>",
+                "");
+        Assert.assertEquals(expected, XMLFormatPreserver.preserve(originalXml, 
newXml, "UTF-8"));
+    }
+
+    @Test
+    public void testRemovedElementDropsItsComments() {
+        String originalXml = String.join("\n",
+                "<Server>",
+                "    <!-- Listener A -->",
+                "    <Listener className=\"org.a.A\"/>",
+                "    <Listener className=\"org.b.B\"/>",
+                "</Server>",
+                "");
+        String newXml = String.join("\n",
+                "<Server>",
+                "  <Listener className=\"org.b.B\"/>",
+                "</Server>",
+                "");
+        String expected = String.join("\n",
+                "<?xml version=\"1.0\" encoding=\"UTF-8\"?>",
+                "<Server>",
+                "  <Listener className=\"org.b.B\"/>",
+                "</Server>",
+                "");
+        Assert.assertEquals(expected, XMLFormatPreserver.preserve(originalXml, 
newXml, "UTF-8"));
+    }
+
+    @Test
+    public void testTextElementsMatchedByText() {
+        String originalXml = String.join("\n",
+                "<Host name=\"localhost\">",
+                "    <!-- watched conf -->",
+                "    <WatchedResource>conf/context.xml</WatchedResource>",
+                "    <WatchedResource>WEB-INF/web.xml</WatchedResource>",
+                "</Host>",
+                "");
+        String newXml = String.join("\n",
+                "<Host name=\"localhost\">",
+                "  <WatchedResource>conf/context.xml</WatchedResource>",
+                "</Host>",
+                "");
+        String expected = String.join("\n",
+                "<?xml version=\"1.0\" encoding=\"UTF-8\"?>",
+                "<Host name=\"localhost\">",
+                "    <!-- watched conf -->",
+                "  <WatchedResource>conf/context.xml</WatchedResource>",
+                "</Host>",
+                "");
+        Assert.assertEquals(expected, XMLFormatPreserver.preserve(originalXml, 
newXml, "UTF-8"));
+    }
+
+    @Test
+    public void testEscaping() {
+        String originalXml = "<Server><Connector port=\"8080\" note=\"a &amp; 
b &lt; c\"/></Server>";
+        String newXml = "<Server><Connector port=\"8080\" note=\"a &amp; b 
&lt; c\"/></Server>";
+        String result = XMLFormatPreserver.preserve(originalXml, newXml, 
"UTF-8");
+        Assert.assertTrue(result, result.contains("note=\"a &amp; b &lt; 
c\""));
+    }
+
+    @Test
+    public void testCrlfLineEndingsPreserved() {
+        String originalXml = "<?xml version=\"1.0\" 
encoding=\"UTF-8\"?>\r\n<!-- comment -->\r\n"
+                + "<Server port=\"8005\"/>\r\n";
+        String newXml = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n<Server 
port=\"8006\"/>\n";
+        String expected = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\r\n<!-- 
comment -->\r\n"
+                + "<Server port=\"8006\"/>\r\n";
+        Assert.assertEquals(expected, XMLFormatPreserver.preserve(originalXml, 
newXml, "UTF-8"));
+    }
+
+    @Test
+    public void testDoctypePreserved() {
+        String originalXml = "<?xml version=\"1.0\"?>\n<!DOCTYPE Server SYSTEM 
\"server.dtd\">\n"
+                + "<Server port=\"8005\"/>\n";
+        String newXml = "<Server port=\"8006\"/>\n";
+        String expected = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n"
+                + "<!DOCTYPE Server SYSTEM \"server.dtd\">\n" + "<Server 
port=\"8006\"/>\n";
+        Assert.assertEquals(expected, XMLFormatPreserver.preserve(originalXml, 
newXml, "UTF-8"));
+    }
+
+    @Test
+    public void testResultIsWellFormed() throws Exception {
+        String originalXml = String.join("\n",
+                "<Server>",
+                "    <!-- comment -->",
+                "    <Service name=\"Catalina\"/>",
+                "</Server>",
+                "");
+        String newXml = String.join("\n",
+                "<Server>",
+                "  <Service name=\"Catalina\"/>",
+                "  <Listener className=\"org.x.Y\"/>",
+                "</Server>",
+                "");
+        String result = XMLFormatPreserver.preserve(originalXml, newXml, 
"UTF-8");
+        SAXParserFactory.newInstance().newSAXParser().getXMLReader().parse(new 
InputSource(new StringReader(result)));
+    }
+
+    @Test
+    public void testIdempotent() {
+        String originalXml = String.join("\n",
+                "<?xml version=\"1.0\" encoding=\"UTF-8\"?>",
+                "<!--",
+                "  License header line 1",
+                "-->",
+                "<Server port=\"8005\">",
+                "    <!-- Section comment -->",
+                "",
+                "    <Service name=\"Catalina\">",
+                "        <Connector port=\"8080\"/>",
+                "    </Service>",
+                "</Server>",
+                "");
+        String newXml = String.join("\n",
+                "<?xml version=\"1.0\" encoding=\"UTF-8\"?>",
+                "<Server port=\"8006\">",
+                "  <Service name=\"Catalina\">",
+                "    <Connector port=\"8080\"/>",
+                "  </Service>",
+                "</Server>",
+                "");
+        String formatted = XMLFormatPreserver.preserve(originalXml, newXml, 
"UTF-8");
+        String formattedAgain = XMLFormatPreserver.preserve(formatted, newXml, 
"UTF-8");
+        Assert.assertEquals(formatted, formattedAgain);
+    }
+
+    @Test
+    public void testFileOverload() throws Exception {
+        String originalXml = String.join("\n",
+                "<Server>",
+                "    <!-- comment -->",
+                "    <Service name=\"Catalina\"/>",
+                "</Server>",
+                "");
+        String newXml = String.join("\n",
+                "<Server>",
+                "  <Service name=\"Catalina\"/>",
+                "</Server>",
+                "");
+        String expected = String.join("\n",
+                "<?xml version=\"1.0\" encoding=\"UTF-8\"?>",
+                "<Server>",
+                "    <!-- comment -->",
+                "  <Service name=\"Catalina\"/>",
+                "</Server>",
+                "");
+        File file = File.createTempFile("xml-format-preserver", ".xml");
+        file.deleteOnExit();
+        Files.write(file.toPath(), 
originalXml.getBytes(StandardCharsets.UTF_8));
+        Assert.assertEquals(expected, XMLFormatPreserver.preserve(file, 
newXml, "UTF-8"));
+        Assert.assertEquals(newXml, XMLFormatPreserver.preserve(new File(file, 
"missing.xml"), newXml, "UTF-8"));
+        Assert.assertEquals(newXml, 
XMLFormatPreserver.preserve(file.getParentFile(), newXml, "UTF-8"));
+    }
+
+    @Test
+    public void testFileOverloadWithBom() throws Exception {
+        String originalXml = "\uFEFF" + "<Server>\n    <!-- comment -->\n    
<Service name=\"Catalina\"/>\n</Server>\n";
+        String newXml = "<Server>\n  <Service 
name=\"Catalina\"/>\n</Server>\n";
+        String expected = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n"
+                + "<Server>\n    <!-- comment -->\n  <Service 
name=\"Catalina\"/>\n</Server>\n";
+        File file = File.createTempFile("xml-format-preserver", ".xml");
+        file.deleteOnExit();
+        Files.write(file.toPath(), 
originalXml.getBytes(StandardCharsets.UTF_8));
+        Assert.assertEquals(expected, XMLFormatPreserver.preserve(file, 
newXml, "UTF-8"));
+    }
+
+}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index a3ffdca1ec..818c4c3d07 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -124,6 +124,12 @@
         <code>Realm</code> instance may not be reused apart from re-adding it 
to
         the same <code>CombinedRealm</code>. (markt)
       </fix>
+      <update>
+        When <code>StoreConfig</code> re-writes an existing
+        <code>server.xml</code> or context configuration file, preserve the
+        layout of the existing file (comments, blank lines and attribute
+        order) on a best effort basis. (remm)
+      </update>
     </changelog>
   </subsection>
 </section>
diff --git a/webapps/docs/config/listeners.xml 
b/webapps/docs/config/listeners.xml
index 8fb5a8bd36..385ca44266 100644
--- a/webapps/docs/config/listeners.xml
+++ b/webapps/docs/config/listeners.xml
@@ -446,6 +446,14 @@
       Before overwriting <code>server.xml</code>, a backup of the existing file
       is created.</p>
 
+    <p>When an existing configuration file is re-written, the generated
+      configuration is reformatted on a best effort basis to preserve the
+      layout of the existing file, including comments, blank lines and the
+      order of attributes. Layout information that cannot be matched to the
+      stored content, for example comments attached to elements that no longer
+      exist, is not preserved. If the layout of the existing file cannot be
+      preserved, the file is written without layout information.</p>
+
     <p>This listener must only be nested within <a 
href="server.html">Server</a>
     elements.</p>
 


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to