#i101955# silently ignore unreadable parts of a corrupted registrymodificaions.xcu; migrate old user layer data (can be removed again once migration is no longer relevant); improved XcuParser to correctly record modifications from old user layer data; localized property values support oor:op "fuse" instead of "modify"

6 files changed, 145 insertions, 69 deletions

diff --git a/configmgr2/source/components.cxx b/configmgr2/source/components.cxx
index c27bdc6ddefa..4cbd9ad7844a 100644
--- a/configmgr2/source/components.cxx
+++ b/configmgr2/source/components.cxx
@@ -374,7 +374,16 @@ Components::Components(
     // potentially fail, anyway, as xcu files in that layer used the xsi
     // namespace prefix without declaring a corresponding namespace binding (see
     // issue 77174)
-    parseModificationLayer();
+    try {
+        parseModificationLayer();
+    } catch (css::uno::Exception & e) { //TODO: more specific exception catching
+        // Silently ignore unreadable parts of a corrupted
+        // registrymodifications.xcu file, instead of completely preventing OOo
+        // from starting:
+        OSL_TRACE(
+            "configmgr error reading user modification layer: %s",
+            rtl::OUStringToOString(e.Message, RTL_TEXTENCODING_UTF8).getStr());
+    }
 }
 
 Components::~Components() {}
@@ -617,6 +626,18 @@ void Components::parseModificationLayer() {
     } catch (css::container::NoSuchElementException &) {
         OSL_TRACE(
             "configmgr user registrymodifications.xcu does not (yet) exist");
+        // Migrate old user layer data (can be removed once migration is no
+        // longer relevant; also see hack for xsi namespace in XmlReader
+        // constructor):
+        parseFiles(
+            Data::NO_LAYER, rtl::OUString(RTL_CONSTASCII_USTRINGPARAM(".xcu")),
+            &parseXcuFile,
+            expand(
+                rtl::OUString(
+                    RTL_CONSTASCII_USTRINGPARAM(
+                        "${$BRAND_BASE_DIR/program/" SAL_CONFIGFILE("bootstrap")
+                        ":UserInstallation}/user/registry/data"))),
+            false);
     }
 }
 
diff --git a/configmgr2/source/writemodfile.cxx b/configmgr2/source/writemodfile.cxx
index f699e99f5e09..baca6f747c68 100644
--- a/configmgr2/source/writemodfile.cxx
+++ b/configmgr2/source/writemodfile.cxx
@@ -447,6 +447,8 @@ void writeModifications(
     rtl::OUString const & nodeName, rtl::Reference< Node > const & node,
     Modifications::Node const & modifications)
 {
+    // It is never necessary to write oor:finalized or oor:mandatory attributes,
+    // as they cannot be set via the UNO API.
     if (modifications.children.empty()) {
         OSL_ASSERT(parent.is());
             // components themselves have no parent but must have children
@@ -460,8 +462,6 @@ void writeModifications(
             writeData(handle, RTL_CONSTASCII_STRINGPARAM("\">"));
             writeNode(components, handle, parent, nodeName, node);
             writeData(handle, RTL_CONSTASCII_STRINGPARAM("</item>"));
-            // It is never necessary to write the oor:mandatory attribute, as it
-            // cannot be set via the UNO API.
         } else {
             writeData(handle, RTL_CONSTASCII_STRINGPARAM("<item oor:path=\""));
             switch (parent->kind()) {
diff --git a/configmgr2/source/xcuparser.cxx b/configmgr2/source/xcuparser.cxx
index 335639def60a..068ebe0e5268 100644
--- a/configmgr2/source/xcuparser.cxx
+++ b/configmgr2/source/xcuparser.cxx
@@ -89,7 +89,7 @@ bool XcuParser::startElement(
         } else if (ns == XmlReader::NAMESPACE_OOR &&
                    name.equals(RTL_CONSTASCII_STRINGPARAM("items")))
         {
-            state_.push(State(rtl::Reference< Node >(), false, false));
+            state_.push(State(rtl::Reference< Node >(), false));
         } else {
             throw css::uno::RuntimeException(
                 (rtl::OUString(
@@ -209,6 +209,7 @@ void XcuParser::endElement(XmlReader const & reader) {
         return;
     }
     OSL_ASSERT(!state_.empty());
+    bool ignore = state_.top().ignore;
     rtl::Reference< Node > insert;
     rtl::OUString name;
     if (state_.top().insert) {
@@ -221,14 +222,23 @@ void XcuParser::endElement(XmlReader const & reader) {
         OSL_ASSERT(!state_.empty() && state_.top().node.is());
         state_.top().node->getMembers()[name] = insert;
     }
+    if (!ignore && !modificationPath_.empty()) {
+        modificationPath_.pop_back();
+            // </item> will pop less than <item> pushed, but that is harmless,
+            // as the next <item> will reset modificationPath_
+    }
 }
 
 void XcuParser::characters(Span const & text) {
     valueParser_.characters(text);
 }
 
-XcuParser::Operation XcuParser::parseOperation(Span const & text) {
-    if (!text.is() || text.equals(RTL_CONSTASCII_STRINGPARAM("modify"))) {
+XcuParser::Operation XcuParser::parseOperation(
+    Span const & text, Operation defaultOperation)
+{
+    if (!text.is()) {
+        return defaultOperation;
+    } else if (text.equals(RTL_CONSTASCII_STRINGPARAM("modify"))) {
         return OPERATION_MODIFY;
     } else if (text.equals(RTL_CONSTASCII_STRINGPARAM("replace"))) {
         return OPERATION_REPLACE;
@@ -306,7 +316,7 @@ void XcuParser::handleComponentData(XmlReader & reader) {
              reader.getUrl()),
             css::uno::Reference< css::uno::XInterface >());
     }
-    switch (parseOperation(attrOp)) {
+    switch (parseOperation(attrOp, OPERATION_MODIFY)) {
     case OPERATION_MODIFY:
     case OPERATION_FUSE:
         break;
@@ -323,7 +333,11 @@ void XcuParser::handleComponentData(XmlReader & reader) {
          ? valueParser_.getLayer() : Data::NO_LAYER),
         node->getFinalized());
     node->setFinalized(finalizedLayer);
-    state_.push(State(node, finalizedLayer < valueParser_.getLayer(), false));
+    state_.push(State(node, finalizedLayer < valueParser_.getLayer()));
+    if (valueParser_.getLayer() == Data::NO_LAYER) {
+        OSL_ASSERT(modificationPath_.empty());
+        modificationPath_.push_back(componentName_);
+    }
 }
 
 void XcuParser::handleItem(XmlReader & reader) {
@@ -350,7 +364,8 @@ void XcuParser::handleItem(XmlReader & reader) {
     int finalizedLayer;
     rtl::Reference< Node > node(
         data_->resolvePathRepresentation(
-            xmldata::convertFromUtf8(attrPath), &pathPrefix_, &finalizedLayer));
+            xmldata::convertFromUtf8(attrPath), &modificationPath_,
+            &finalizedLayer));
     if (!node.is()) {
         throw css::uno::RuntimeException(
             (rtl::OUString(
@@ -358,16 +373,15 @@ void XcuParser::handleItem(XmlReader & reader) {
              reader.getUrl()),
             css::uno::Reference< css::uno::XInterface >());
     }
-    OSL_ASSERT(!pathPrefix_.empty());
-    componentName_ = pathPrefix_.front();
-    state_.push(
-        State(
-            node, finalizedLayer < valueParser_.getLayer(),
-            valueParser_.getLayer() == Data::NO_LAYER));
+    OSL_ASSERT(!modificationPath_.empty());
+    componentName_ = modificationPath_.front();
+    if (valueParser_.getLayer() != Data::NO_LAYER) {
+        modificationPath_.clear();
+    }
+    state_.push(State(node, finalizedLayer < valueParser_.getLayer()));
 }
 
 void XcuParser::handlePropValue(XmlReader & reader, PropertyNode * prop) {
-    OSL_ASSERT(!state_.top().record);
     Span attrNil;
     Span attrSeparator;
     Span attrExternal;
@@ -431,7 +445,6 @@ void XcuParser::handlePropValue(XmlReader & reader, PropertyNode * prop) {
 void XcuParser::handleLocpropValue(
     XmlReader & reader, LocalizedPropertyNode * locprop)
 {
-    OSL_ASSERT(!state_.top().record);
     Span attrLang;
     Span attrNil;
     Span attrSeparator;
@@ -480,8 +493,8 @@ void XcuParser::handleLocpropValue(
              reader.getUrl()),
             css::uno::Reference< css::uno::XInterface >());
     }
-    switch (parseOperation(attrOp)) {
-    case OPERATION_MODIFY:
+    switch (parseOperation(attrOp, OPERATION_FUSE)) {
+    case OPERATION_FUSE:
         if (nil) {
             if (i == locprop->getMembers().end()) {
                 locprop->getMembers()[name] = new LocalizedValueNode(
@@ -495,6 +508,11 @@ void XcuParser::handleLocpropValue(
             valueParser_.separator_ = attrSeparator;
             valueParser_.start(locprop, name);
         }
+        if (!modificationPath_.empty()) {
+            modificationPath_.push_back(name);
+            data_->modifications.add(modificationPath_);
+            modificationPath_.pop_back();
+        }
         break;
     case OPERATION_REMOVE:
         //TODO: only allow if parent.op == OPERATION_FUSE
@@ -503,6 +521,11 @@ void XcuParser::handleLocpropValue(
             locprop->getMembers().erase(i);
         }
         state_.push(State());
+        if (!modificationPath_.empty()) {
+            modificationPath_.push_back(name);
+            data_->modifications.add(modificationPath_);
+            modificationPath_.pop_back();
+        }
         break;
     default:
         throw css::uno::RuntimeException(
@@ -551,11 +574,6 @@ void XcuParser::handleGroupProp(XmlReader & reader, GroupNode * group) {
             css::uno::Reference< css::uno::XInterface >());
     }
     rtl::OUString name(xmldata::convertFromUtf8(attrName));
-    if (state_.top().record) {
-        Path path(pathPrefix_);
-        path.push_back(name);
-        data_->modifications.add(path);
-    }
     Type type = xmldata::parseType(reader, attrType);
     if (type == TYPE_ANY) {
         throw css::uno::RuntimeException(
@@ -565,7 +583,7 @@ void XcuParser::handleGroupProp(XmlReader & reader, GroupNode * group) {
              reader.getUrl()),
             css::uno::Reference< css::uno::XInterface >());
     }
-    Operation op = parseOperation(attrOp);
+    Operation op = parseOperation(attrOp, OPERATION_MODIFY);
     bool finalized = xmldata::parseBoolean(attrFinalized, false);
     NodeMap::iterator i(group->getMembers().find(name));
     if (i == group->getMembers().end()) {
@@ -633,7 +651,11 @@ void XcuParser::handleUnknownGroupProp(
             if (finalized) {
                 prop->setFinalized(valueParser_.getLayer());
             }
-            state_.push(State(prop, name, state_.top().locked, false));
+            state_.push(State(prop, name, state_.top().locked));
+            if (!modificationPath_.empty()) {
+                modificationPath_.push_back(name);
+                data_->modifications.add(modificationPath_);
+            }
         }
         break;
     case OPERATION_REMOVE:
@@ -676,8 +698,12 @@ void XcuParser::handlePlainGroupProp(
         state_.push(
             State(
                 property,
-                state_.top().locked || finalizedLayer < valueParser_.getLayer(),
-                false));
+                (state_.top().locked ||
+                 finalizedLayer < valueParser_.getLayer())));
+        if (!modificationPath_.empty()) {
+            modificationPath_.push_back(name);
+            data_->modifications.add(modificationPath_);
+        }
         break;
     case OPERATION_REMOVE:
         if (!property->isExtension()) {
@@ -691,6 +717,11 @@ void XcuParser::handlePlainGroupProp(
         }
         group->getMembers().erase(propertyIndex);
         state_.push(State()); // ignore children
+        if (!modificationPath_.empty()) {
+            modificationPath_.push_back(name);
+            data_->modifications.add(modificationPath_);
+            modificationPath_.pop_back();
+        }
         break;
     }
 }
@@ -724,8 +755,11 @@ void XcuParser::handleLocalizedGroupProp(
         state_.push(
             State(
                 property,
-                state_.top().locked || finalizedLayer < valueParser_.getLayer(),
-                false));
+                (state_.top().locked ||
+                 finalizedLayer < valueParser_.getLayer())));
+        if (!modificationPath_.empty()) {
+            modificationPath_.push_back(name);
+        }
         break;
     case OPERATION_REPLACE:
         {
@@ -738,8 +772,11 @@ void XcuParser::handleLocalizedGroupProp(
                 State(
                     replacement, name,
                     (state_.top().locked ||
-                     finalizedLayer < valueParser_.getLayer()),
-                    false));
+                     finalizedLayer < valueParser_.getLayer())));
+            if (!modificationPath_.empty()) {
+                modificationPath_.push_back(name);
+                data_->modifications.add(modificationPath_);
+            }
         }
         break;
     case OPERATION_REMOVE:
@@ -756,7 +793,6 @@ void XcuParser::handleLocalizedGroupProp(
 void XcuParser::handleGroupNode(
     XmlReader & reader, rtl::Reference< Node > const & group)
 {
-    OSL_ASSERT(!state_.top().record);
     Span attrName;
     Span attrOp;
     Span attrFinalized;
@@ -788,18 +824,17 @@ void XcuParser::handleGroupNode(
             css::uno::Reference< css::uno::XInterface >());
     }
     rtl::OUString name(xmldata::convertFromUtf8(attrName));
-    rtl::Reference< Node > subgroup(
+    rtl::Reference< Node > child(
         Data::findNode(valueParser_.getLayer(), group->getMembers(), name));
-    if (!subgroup.is()) {
+    if (!child.is()) {
         throw css::uno::RuntimeException(
             (rtl::OUString(RTL_CONSTASCII_USTRINGPARAM("unknown node ")) +
              name + rtl::OUString(RTL_CONSTASCII_USTRINGPARAM(" in ")) +
              reader.getUrl()),
             css::uno::Reference< css::uno::XInterface >());
     }
-    Operation op(parseOperation(attrOp));
-    if ((op != OPERATION_MODIFY && op != OPERATION_FUSE) || state_.top().record)
-    {
+    Operation op(parseOperation(attrOp, OPERATION_MODIFY));
+    if (op != OPERATION_MODIFY && op != OPERATION_FUSE) {
         throw css::uno::RuntimeException(
             (rtl::OUString(
                 RTL_CONSTASCII_USTRINGPARAM(
@@ -810,13 +845,15 @@ void XcuParser::handleGroupNode(
     int finalizedLayer = std::min(
         (xmldata::parseBoolean(attrFinalized, false)
          ? valueParser_.getLayer() : Data::NO_LAYER),
-        subgroup->getFinalized());
-    subgroup->setFinalized(finalizedLayer);
+        child->getFinalized());
+    child->setFinalized(finalizedLayer);
     state_.push(
         State(
-            subgroup,
-            state_.top().locked || finalizedLayer < valueParser_.getLayer(),
-            false));
+            child,
+            state_.top().locked || finalizedLayer < valueParser_.getLayer()));
+    if (!modificationPath_.empty()) {
+        modificationPath_.push_back(name);
+    }
 }
 
 void XcuParser::handleSetNode(XmlReader & reader, SetNode * set) {
@@ -870,11 +907,6 @@ void XcuParser::handleSetNode(XmlReader & reader, SetNode * set) {
         xmldata::parseTemplateReference(
             attrComponent, attrNodeType, componentName_,
             &set->getDefaultTemplateName()));
-    if (state_.top().record) {
-        Path path(pathPrefix_);
-        path.push_back(name);
-        data_->modifications.add(path);
-    }
     if (!set->isValidTemplate(templateName)) {
         throw css::uno::RuntimeException(
             (rtl::OUString(RTL_CONSTASCII_USTRINGPARAM("set member node ")) +
@@ -898,7 +930,7 @@ void XcuParser::handleSetNode(XmlReader & reader, SetNode * set) {
              reader.getUrl()),
             css::uno::Reference< css::uno::XInterface >());
     }
-    Operation op(parseOperation(attrOp));
+    Operation op(parseOperation(attrOp, OPERATION_MODIFY));
     int finalizedLayer = xmldata::parseBoolean(attrFinalized, false)
         ? valueParser_.getLayer() : Data::NO_LAYER;
     int mandatoryLayer = xmldata::parseBoolean(attrMandatory, false)
@@ -928,8 +960,11 @@ void XcuParser::handleSetNode(XmlReader & reader, SetNode * set) {
         state_.push(
             State(
                 i->second,
-                state_.top().locked || finalizedLayer < valueParser_.getLayer(),
-                false));
+                (state_.top().locked ||
+                 finalizedLayer < valueParser_.getLayer())));
+        if (!modificationPath_.empty()) {
+            modificationPath_.push_back(name);
+        }
         break;
     case OPERATION_REPLACE:
         if (state_.top().locked || finalizedLayer < valueParser_.getLayer()) {
@@ -939,7 +974,11 @@ void XcuParser::handleSetNode(XmlReader & reader, SetNode * set) {
             member->setLayer(valueParser_.getLayer());
             member->setFinalized(finalizedLayer);
             member->setMandatory(mandatoryLayer);
-            state_.push(State(member, name, false, false));
+            state_.push(State(member, name, false));
+            if (!modificationPath_.empty()) {
+                modificationPath_.push_back(name);
+                data_->modifications.add(modificationPath_);
+            }
         }
         break;
     case OPERATION_FUSE:
@@ -952,15 +991,21 @@ void XcuParser::handleSetNode(XmlReader & reader, SetNode * set) {
                 member->setLayer(valueParser_.getLayer());
                 member->setFinalized(finalizedLayer);
                 member->setMandatory(mandatoryLayer);
-                state_.push(State(member, name, false, false));
+                state_.push(State(member, name, false));
+                if (!modificationPath_.empty()) {
+                    modificationPath_.push_back(name);
+                    data_->modifications.add(modificationPath_);
+                }
             }
         } else {
             state_.push(
                 State(
                     i->second,
                     (state_.top().locked ||
-                     finalizedLayer < valueParser_.getLayer()),
-                    false));
+                     finalizedLayer < valueParser_.getLayer())));
+            if (!modificationPath_.empty()) {
+                modificationPath_.push_back(name);
+            }
         }
         break;
     case OPERATION_REMOVE:
@@ -973,6 +1018,11 @@ void XcuParser::handleSetNode(XmlReader & reader, SetNode * set) {
             set->getMembers().erase(i);
         }
         state_.push(State());
+        if (!modificationPath_.empty()) {
+            modificationPath_.push_back(name);
+            data_->modifications.add(modificationPath_);
+            modificationPath_.pop_back();
+        }
         break;
     }
 }
diff --git a/configmgr2/source/xcuparser.hxx b/configmgr2/source/xcuparser.hxx
index 25b78ddb8c16..23ceb6cdb3d6 100644
--- a/configmgr2/source/xcuparser.hxx
+++ b/configmgr2/source/xcuparser.hxx
@@ -74,7 +74,8 @@ private:
     enum Operation {
         OPERATION_MODIFY, OPERATION_REPLACE, OPERATION_FUSE, OPERATION_REMOVE };
 
-    static Operation parseOperation(Span const & text);
+    static Operation parseOperation(
+        Span const & text, Operation defaultOperation);
 
     void handleComponentData(XmlReader & reader);
 
@@ -112,22 +113,19 @@ private:
         bool ignore;
         bool insert;
         bool locked;
-        bool record;
 
-        inline State():
-            ignore(true), insert(false), locked(false), record(false) {}
+        inline State(): ignore(true), insert(false), locked(false) {}
 
-        inline State(
-            rtl::Reference< Node > const & theNode, bool theLocked,
-            bool theRecord):
-            node(theNode), ignore(false), insert(false), locked(theLocked),
-            record(theRecord) {}
+        inline State(rtl::Reference< Node > const & theNode, bool theLocked):
+            node(theNode), ignore(false), insert(false), locked(theLocked)
+        {}
 
         inline State(
             rtl::Reference< Node > const & theNode,
-            rtl::OUString const & theName, bool theLocked, bool theRecord):
+            rtl::OUString const & theName, bool theLocked):
             node(theNode), name(theName), ignore(false), insert(true),
-            locked(theLocked), record(theRecord) {}
+            locked(theLocked)
+        {}
     };
 
     typedef std::stack< State > StateStack;
@@ -136,7 +134,7 @@ private:
     Data * data_;
     rtl::OUString componentName_;
     StateStack state_;
-    Path pathPrefix_;
+    Path modificationPath_;
     rtl::OUString path_;
 };
 
diff --git a/configmgr2/source/xmlreader.cxx b/configmgr2/source/xmlreader.cxx
index cc7fbfc6990b..2bac273610fb 100644
--- a/configmgr2/source/xmlreader.cxx
+++ b/configmgr2/source/xmlreader.cxx
@@ -103,6 +103,13 @@ XmlReader::XmlReader(rtl::OUString const & fileUrl)
     }
     namespaces_.push_back(
         NamespaceData(Span(RTL_CONSTASCII_STRINGPARAM("xml")), NAMESPACE_XML));
+    namespaces_.push_back(
+        NamespaceData(Span(RTL_CONSTASCII_STRINGPARAM("xsi")), NAMESPACE_XSI));
+        // old user layer .xcu files used the xsi namespace prefix without
+        // declaring a corresponding namespace binding, see issue 77174; reading
+        // those files during migration would fail without this hack that can be
+        // removed once migration is no longer relevant (see
+        // Components::parseModificationLayer)
     pos_ = static_cast< char * >(fileAddress_);
     end_ = pos_ + fileSize_;
     state_ = STATE_CONTENT;
diff --git a/officecfg/registry/component-update.dtd b/officecfg/registry/component-update.dtd
index 1781f06f11fa..6b44c2066dee 100644
--- a/officecfg/registry/component-update.dtd
+++ b/officecfg/registry/component-update.dtd
@@ -71,14 +71,14 @@
              oor:string-list -->
 <!ATTLIST value
     xml:lang CDATA #IMPLIED
-    oor:op (modify | remove) #IMPLIED
+    oor:op (fuse | remove) #IMPLIED
     xsi:nil (true | false) #IMPLIED
     oor:separator CDATA #IMPLIED
     oor:external CDATA #IMPLIED
     install:module CDATA #IMPLIED>
     <!-- xml:lang  must only be used for localized properties; defaults to the
              empty string
-         oor:op  defaults to "modify"; "remove" must only be used for localized
+         oor:op  defaults to "fuse"; "remove" must only be used for localized
              properties
          xsi:nil  defaults to "false"; "true" must only be used for nillable
              properties