[Libreoffice-commits] core.git: configmgr/qa configmgr/source

2023-05-11 Thread Stephan Bergmann (via logerrit)
 configmgr/qa/unit/test.cxx |   23 +--
 configmgr/source/data.cxx  |   11 +++
 2 files changed, 24 insertions(+), 10 deletions(-)

New commits:
commit abd630e81bc150d05e4129cc22752ecf461777c7
Author: Stephan Bergmann 
AuthorDate: Thu May 11 14:39:19 2023 +0200
Commit: Stephan Bergmann 
CommitDate: Thu May 11 16:49:00 2023 +0200

Allow all hierarchical path segments to be ['...'] quoted

...and not just set elements.  

"tdf#104005 Don't allow changing finalized properties" found that some 
existing
property names include "/", which makes them unusable with e.g.
getPropertyByHierarchicalName.

The most obvious solution is to allow ['...'] quoting (without a leading
template name, though) for all kinds of hierarchical path segments.  (In 
theory,
that would cause backwards incompatibility, as e.g. a property named ['foo']
could no longer be referenced by a ['foo'] path segment (it would need to be
referenced by a ['[foo]'] path segment instead).  But in 
practice,
that path segment ['foo'] would have been rejected in the past anyway, as 
it did
not reference a set element.)

To make this work, the meaning of the setElement out-parameter of
configmgr::Data::parseSegment is changed to only be true if the segment uses
['...'] notation including a leading (non-empty) template name.

What this change does not (yet) address is writing out such quoted (group or
set) names in the hierarchical paths written out in configmgr::writeModFile,
where necessary.  (It never writes out names of properties as parts of
hierarchical names, so this wouldn't be an issue for the existing 
problematic
properties containing "/" in their names, anyway.)

Change-Id: I635d823c7bbb6b8ac5869c7e0130ba8cfd329599
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/151673
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann 

diff --git a/configmgr/qa/unit/test.cxx b/configmgr/qa/unit/test.cxx
index 60d19be8472c..aa5b137413d3 100644
--- a/configmgr/qa/unit/test.cxx
+++ b/configmgr/qa/unit/test.cxx
@@ -205,12 +205,23 @@ void Test::setUp()
 
 void Test::testKeyFetch()
 {
-OUString s;
-CPPUNIT_ASSERT(
-getKey(
-"/org.openoffice.System",
-"L10N/Locale") >>=
-s);
+{
+OUString s;
+CPPUNIT_ASSERT(
+getKey(
+"/org.openoffice.System",
+"L10N/Locale") >>=
+s);
+}
+{
+auto const v = getKey(
+"/org.openoffice.Office.Embedding",
+
"MimeTypeClassIDRelations/['application/vnd.sun.xml.report.chart']");
+OUString s;
+CPPUNIT_ASSERT(v >>= s);
+CPPUNIT_ASSERT_EQUAL(OUString("80243D39-6741-46C5-926E-069164FF87BB"), 
s);
+// cf. officecfg/registry/data/org/openoffice/Office/Embedding.xcu
+}
 }
 
 void Test::testKeySet()
diff --git a/configmgr/source/data.cxx b/configmgr/source/data.cxx
index 24987d65c44b..b0cad8042962 100644
--- a/configmgr/source/data.cxx
+++ b/configmgr/source/data.cxx
@@ -124,10 +124,14 @@ sal_Int32 Data::parseSegment(
 *setElement = false;
 return i;
 }
-if (templateName != nullptr) {
-if (i - index == 1 && path[index] == '*') {
+if (i - index == 1 && path[index] == '*') {
+*setElement = true;
+if (templateName != nullptr) {
 templateName->clear();
-} else {
+}
+} else {
+*setElement = i != index;
+if (templateName != nullptr) {
 *templateName = path.copy(index, i - index);
 }
 }
@@ -144,7 +148,6 @@ sal_Int32 Data::parseSegment(
 {
 return -1;
 }
-*setElement = true;
 return j + 2;
 }
 


[Libreoffice-commits] core.git: configmgr/qa configmgr/source

2023-02-15 Thread Stephan Bergmann (via logerrit)
 configmgr/qa/unit/test.cxx  |7 +++
 configmgr/source/access.cxx |   12 ++--
 2 files changed, 13 insertions(+), 6 deletions(-)

New commits:
commit c3bd52f81bf733a0b9b0560794a54b2ac1e0f444
Author: Stephan Bergmann 
AuthorDate: Wed Feb 15 21:22:51 2023 +0100
Commit: Stephan Bergmann 
CommitDate: Wed Feb 15 23:24:12 2023 +

Use the (first segment of the) original locale value for the workaround 
again

cf7c9599e776eba8e14614cecb528d3da5778190 "Make comphelper/configuration.hxx 
work
for localized properties", which had originally introduced this code, had 
been
careful to ensure that the

> assert(
> !locale.isEmpty() && locale.indexOf('-') == -1 &&
> locale.indexOf('_') == -1);

would always hold here (it had already removed all trailing "-..." and 
"_..."
segments, but had made sure to stop before `locale`, which is known to 
initially
be no non-empty, would have become empty, by treating a leading "-" or "_" 
not
as a segment delimiter, but rather as a first segment).

dfc28be2487c13be36a90efd778b8d8f179c589d "configmgr: Use a proper
LanguageTag-based locale fallback mechanism" had changed that, instead 
setting
`locale` to some value obtained from `LanguageTag::getFallbackStrings`, 
which
might or might not satisfy the assert.

But there was no good reason for that part of
dfc28be2487c13be36a90efd778b8d8f179c589d in the first place:  The 
workaround (as
explained in the leading code comment) was meant to be carried out with the
first segment of the original `locale` value, not with some fallback value. 
 So
put back here the computation of that first segment of the original `locale`
value.  (And drop the misleading empty line that
dfc28be2487c13be36a90efd778b8d8f179c589d had, for no good reason, introduced
between the workaround's leading code comment and its actual code.)

However, it turns out that there was one flaw in
cf7c9599e776eba8e14614cecb528d3da5778190:  When the original `locale` starts
with "-" or "_", the resulting `locale` representing the first segment will 
be
"-" or "_", so the `locale.indexOf('-') == -1 && locale.indexOf('_') == -1` 
part
of the assert would be false.  But that wouldn't be an issue for the 
following
code (the only issue would be if `locale` had become empty, in which case 
the
`name2.startsWiht(locale)` check would trivially become true, and that for 
loop
would erroneously pick the child with the empty `name2`), and that part of 
the
assert had merely been there to reinforce that `locale` had indeed been 
stripped
down to the first segment.  A correct version of the assert would have used
`locale.indexOf('-', 1) == -1 && locale.indexOf('_', 1) == -1` instead, but 
as
the code now makes it obvious anyway that `locale` has been cut down here 
to the
first segment, we can just as well simplify the assert to just the
`!locale.isEmpty()` part.

(The added test code unfortunately doesn't actually test this piece of 
code, and
somewhat unexpectedly receives the "default" value from the empty string 
locale
default, rather than the "en-US" value from the higher precedence "en-US" 
locale
default, because `aFallbacks` happens to contain an empty string, so we 
already
leave Access::getChild early in the "Find the best match using the 
LanguageTag
fallback mechanism, excluding the original tag" block.)

Change-Id: Ib92e714c9db4879be058529ec905e631df975424
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/147113
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann 

diff --git a/configmgr/qa/unit/test.cxx b/configmgr/qa/unit/test.cxx
index 3de93a6672df..7aa2daf6f96d 100644
--- a/configmgr/qa/unit/test.cxx
+++ b/configmgr/qa/unit/test.cxx
@@ -324,6 +324,13 @@ void Test::testLocalizedProperty() {
 
access->getByHierarchicalName("/org.libreoffice.unittest/localized/*pt") >>= v);
 CPPUNIT_ASSERT_EQUAL(OUString("pt-PT"), v);
 }
+{
+// Make sure a degenerate passed-in "-" locale is handled gracefully:
+OUString v;
+CPPUNIT_ASSERT(
+
access->getByHierarchicalName("/org.libreoffice.unittest/localized/*-") >>= v);
+CPPUNIT_ASSERT_EQUAL(OUString("default"), v);
+}
 }
 
 void Test::testReadCommands()
diff --git a/configmgr/source/access.cxx b/configmgr/source/access.cxx
index 48d9b46ddc26..9d71c7fa2978 100644
--- a/configmgr/source/access.cxx
+++ b/configmgr/source/access.cxx
@@ -68,6 +68,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -1407,12 +1408,11 @@ rtl::Reference< ChildAccess > Access::getChild(OUString 
const & name) {
 // xml:lang attributes, look for the first entry with the same 
first
 // segment as the requested language tag before falling back to
   

[Libreoffice-commits] core.git: configmgr/qa configmgr/source connectivity/source

2020-05-18 Thread Noel Grandin (via logerrit)
 configmgr/qa/unit/test.cxx|6 ++---
 configmgr/source/dconf.cxx|   16 +++---
 configmgr/source/writemodfile.cxx |   10 
 connectivity/source/commontools/dbtools.cxx   |   13 +--
 connectivity/source/drivers/firebird/DatabaseMetaData.cxx |8 +++
 connectivity/source/drivers/flat/ETable.cxx   |2 -
 connectivity/source/drivers/postgresql/pq_connection.cxx  |8 +++
 connectivity/source/drivers/postgresql/pq_statement.cxx   |8 +++
 connectivity/source/drivers/postgresql/pq_xbase.cxx   |8 +++
 9 files changed, 39 insertions(+), 40 deletions(-)

New commits:
commit 454eb3bc05f861712bff0f7593f9aa9809e4ee7c
Author: Noel Grandin 
AuthorDate: Mon May 18 09:18:34 2020 +0200
Commit: Noel Grandin 
CommitDate: Mon May 18 13:18:38 2020 +0200

use for-range on Sequence in cli_ure..connectivity

Change-Id: Ic5254e402d153a13c29928b59738cbe1603d0139
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/94399
Tested-by: Jenkins
Reviewed-by: Noel Grandin 

diff --git a/configmgr/qa/unit/test.cxx b/configmgr/qa/unit/test.cxx
index e553c1947f66..daa2070086f7 100644
--- a/configmgr/qa/unit/test.cxx
+++ b/configmgr/qa/unit/test.cxx
@@ -316,15 +316,15 @@ void Test::testReadCommands()
 "/org.openoffice.Office.UI.GenericCommands/UserInterface/"
  "Commands"),
 css::uno::UNO_QUERY_THROW);
-css::uno::Sequence< OUString > names(access->getElementNames());
+const css::uno::Sequence< OUString > names(access->getElementNames());
 
 /*CPPUNIT_ASSERT_EQUAL(749, names.getLength());*/
 // testSetSetMemberName() already removed ".uno:FontworkGalleryFloater"
 sal_uInt32 n = osl_getGlobalTimer();
 for (int i = 0; i < 8; ++i) {
-for (sal_Int32 j = 0; j < names.getLength(); ++j) {
+for (OUString const & childName : names) {
 css::uno::Reference< css::container::XNameAccess > child;
-if (access->getByName(names[j]) >>= child) {
+if (access->getByName(childName) >>= child) {
 CPPUNIT_ASSERT(child.is());
 child->getByName("Label");
 child->getByName("ContextLabel");
diff --git a/configmgr/source/dconf.cxx b/configmgr/source/dconf.cxx
index 8493b3351e83..75c0bb360ce3 100644
--- a/configmgr/source/dconf.cxx
+++ b/configmgr/source/dconf.cxx
@@ -1264,12 +1264,12 @@ bool addProperty(
 }
 case TYPE_STRING_LIST:
 {
-css::uno::Sequence seq(
+const css::uno::Sequence seq(
 value.get>());
 std::vector vs;
-for (sal_Int32 i = 0; i != seq.getLength(); ++i) {
+for (OUString const & s : seq) {
 children.emplace_front(
-g_variant_new_string(encodeString(seq[i]).getStr()));
+g_variant_new_string(encodeString(s).getStr()));
 if (children.front().get() == nullptr) {
 SAL_WARN(
 "configmgr.dconf", "g_variant_new_string failed");
@@ -1287,11 +1287,11 @@ bool addProperty(
 }
 case TYPE_HEXBINARY_LIST:
 {
-css::uno::Sequence> seq(
+const css::uno::Sequence> seqSeq(
 value.get<
 css::uno::Sequence>>());
 std::vector vs;
-for (sal_Int32 i = 0; i != seq.getLength(); ++i) {
+for (css::uno::Sequence const & seq : seqSeq) {
 static_assert(
 sizeof(sal_Int32) <= sizeof(gsize),
 "G_MAXSIZE too small");
@@ -1299,8 +1299,8 @@ bool addProperty(
 sizeof (sal_Int8) == sizeof (guchar), "size mismatch");
 children.emplace_front(
 g_variant_new_fixed_array(
-G_VARIANT_TYPE_BYTE, seq[i].getConstArray(),
-seq[i].getLength(), sizeof (sal_Int8)));
+G_VARIANT_TYPE_BYTE, seq.getConstArray(),
+seq.getLength(), sizeof (sal_Int8)));
 if (children.front().get() == nullptr) {
 SAL_WARN(
 "configmgr.dconf",
@@ -1318,7 +1318,7 @@ bool addProperty(
 sizeof(sal_Int32) <= sizeof(gsize),
 "G_MAXSIZE too small");
 v.reset(
-g_variant_new_array(ty.get(), vs.data(), seq.getLength()));
+g_variant_new_array(ty.get(), vs.data(), 
seqSeq.getLength()));
 break;
 }
 default:
diff --git a/configmgr/source/writemodfile.cxx 
b/configmgr/source/writemodfile.cxx
index 

[Libreoffice-commits] core.git: configmgr/qa configmgr/source

2015-07-13 Thread Stephan Bergmann
 configmgr/qa/unit/test.cxx  |5 +++--
 configmgr/source/access.cxx |   12 ++--
 2 files changed, 9 insertions(+), 8 deletions(-)

New commits:
commit e80df0d58ba32e3041ff9c8cdea9c617ea7e633b
Author: Stephan Bergmann sberg...@redhat.com
Date:   Mon Jul 13 14:07:38 2015 +0200

tdf#92639: Slashes are allowed in set member names, of course

Change-Id: I30944fe9611e83566c891a7d1461ad02979daddd

diff --git a/configmgr/qa/unit/test.cxx b/configmgr/qa/unit/test.cxx
index 544a808..a9f609b 100644
--- a/configmgr/qa/unit/test.cxx
+++ b/configmgr/qa/unit/test.cxx
@@ -315,8 +315,9 @@ void Test::testInsertSetMember() {
 } catch (css::lang::IllegalArgumentException ) {}
 try {
 access-insertByName(a/b, css::uno::makeAny(member));
-CPPUNIT_FAIL(expected IllegalArgumentException);
-} catch (css::lang::IllegalArgumentException ) {}
+} catch (css::lang::IllegalArgumentException ) {
+CPPUNIT_FAIL(unexpected IllegalArgumentException);
+}
 css::uno::Referencecss::util::XChangesBatch(
 access, css::uno::UNO_QUERY_THROW)-commitChanges();
 css::uno::Referencecss::lang::XComponent(
diff --git a/configmgr/source/access.cxx b/configmgr/source/access.cxx
index b8fadbc..fa2930a 100644
--- a/configmgr/source/access.cxx
+++ b/configmgr/source/access.cxx
@@ -109,12 +109,12 @@ namespace {
 // Conservatively forbid what is either not an XML Char (including lone
 // surrogates, even though they should not appear in well-formed UNO OUString
 // instances anyway), or is a slash (as it causes problems in path syntax):
-bool isValidName(OUString const  name) {
+bool isValidName(OUString const  name, bool setMember) {
 for (sal_Int32 i = 0; i != name.getLength();) {
 sal_uInt32 c = name.iterateCodePoints(i);
 if ((c  0x20  !(c == 0x09 || c == 0x0A || c == 0x0D))
 || rtl::isHighSurrogate(c) || rtl::isLowSurrogate(c) || c == 0xFFFE
-|| c == 0x || c == '/')
+|| c == 0x || (!setMember  c == '/'))
 {
 return false;
 }
@@ -669,7 +669,7 @@ void Access::setName(OUString const  aName)
 if (node-getMandatory() == Data::NO_LAYER 
 !(other.is()  other-isFinalized()))
 {
-if (!isValidName(aName)) {
+if (!isValidName(aName, true)) {
 throw css::uno::RuntimeException(
 invalid element name  + aName);
 }
@@ -1188,7 +1188,7 @@ void Access::insertByName(
 Modifications localMods;
 switch (getNode()-kind()) {
 case Node::KIND_LOCALIZED_PROPERTY:
-if (!isValidName(aName)) {
+if (!isValidName(aName, false)) {
 throw css::lang::IllegalArgumentException(
 aName, static_castcppu::OWeakObject *(this), 0);
 }
@@ -1196,7 +1196,7 @@ void Access::insertByName(
 break;
 case Node::KIND_GROUP:
 {
-if (!isValidName(aName)) {
+if (!isValidName(aName, false)) {
 throw css::lang::IllegalArgumentException(
 aName, static_castcppu::OWeakObject *(this), 0);
 }
@@ -1212,7 +1212,7 @@ void Access::insertByName(
 break;
 case Node::KIND_SET:
 {
-if (!isValidName(aName)) {
+if (!isValidName(aName, true)) {
 throw css::lang::IllegalArgumentException(
 aName, static_castcppu::OWeakObject *(this), 0);
 }
___
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice-commits


[Libreoffice-commits] core.git: configmgr/qa configmgr/source

2015-07-07 Thread Stephan Bergmann
 configmgr/qa/unit/test.cxx  |   37 +
 configmgr/source/access.cxx |   37 +
 2 files changed, 74 insertions(+)

New commits:
commit 375f9460d99a0e2c366318edcc41d64d6170286e
Author: Stephan Bergmann sberg...@redhat.com
Date:   Tue Jul 7 19:09:03 2015 +0200

Validate names of elements added via the API

Change-Id: I052f8ca6a8788665acb1bf87456f7cc67d64c365

diff --git a/configmgr/qa/unit/test.cxx b/configmgr/qa/unit/test.cxx
index fd00477..a6954ad 100644
--- a/configmgr/qa/unit/test.cxx
+++ b/configmgr/qa/unit/test.cxx
@@ -28,11 +28,13 @@
 #include com/sun/star/beans/XPropertyState.hpp
 #include com/sun/star/configuration/theDefaultProvider.hpp
 #include com/sun/star/container/XHierarchicalNameAccess.hpp
+#include com/sun/star/container/XNameContainer.hpp
 #include com/sun/star/container/XNameReplace.hpp
 #include com/sun/star/container/XNamed.hpp
 #include com/sun/star/lang/EventObject.hpp
 #include com/sun/star/lang/XComponent.hpp
 #include com/sun/star/lang/XMultiServiceFactory.hpp
+#include com/sun/star/lang/XSingleServiceFactory.hpp
 #include com/sun/star/uno/Any.hxx
 #include com/sun/star/uno/Reference.hxx
 #include com/sun/star/uno/RuntimeException.hpp
@@ -66,6 +68,7 @@ public:
 void testKeySet();
 void testKeyReset();
 void testSetSetMemberName();
+void testInsertSetMember();
 void testReadCommands();
 #if 0
 void testThreads();
@@ -93,6 +96,7 @@ public:
 CPPUNIT_TEST(testKeySet);
 CPPUNIT_TEST(testKeyReset);
 CPPUNIT_TEST(testSetSetMemberName);
+CPPUNIT_TEST(testInsertSetMember);
 CPPUNIT_TEST(testReadCommands);
 #if 0
 CPPUNIT_TEST(testThreads);
@@ -286,6 +290,39 @@ void Test::testSetSetMemberName()
 CPPUNIT_ASSERT( s == Fontwork Gallery... );
 }
 
+void Test::testInsertSetMember() {
+css::uno::Referencecss::container::XNameContainer access(
+createUpdateAccess(
+
/org.openoffice.Office.UI.GenericCommands/UserInterface/Commands),
+css::uno::UNO_QUERY_THROW);
+css::uno::Referencecss::uno::XInterface member;
+member.set(
+css::uno::Referencecss::lang::XSingleServiceFactory(
+access, css::uno::UNO_QUERY_THROW)-createInstance());
+CPPUNIT_ASSERT(member.is());
+access-insertByName(A, css::uno::makeAny(member));
+member.set(
+css::uno::Referencecss::lang::XSingleServiceFactory(
+access, css::uno::UNO_QUERY_THROW)-createInstance());
+CPPUNIT_ASSERT(member.is());
+try {
+access-insertByName(, css::uno::makeAny(member));
+CPPUNIT_FAIL(expected IllegalArgumentException);
+} catch (css::lang::IllegalArgumentException ) {}
+try {
+access-insertByName(\x01, css::uno::makeAny(member));
+CPPUNIT_FAIL(expected IllegalArgumentException);
+} catch (css::lang::IllegalArgumentException ) {}
+try {
+access-insertByName(a/b, css::uno::makeAny(member));
+CPPUNIT_FAIL(expected IllegalArgumentException);
+} catch (css::lang::IllegalArgumentException ) {}
+css::uno::Referencecss::util::XChangesBatch(
+access, css::uno::UNO_QUERY_THROW)-commitChanges();
+css::uno::Referencecss::lang::XComponent(
+access, css::uno::UNO_QUERY_THROW)-dispose();
+}
+
 void Test::testReadCommands()
 {
 css::uno::Reference css::container::XNameAccess  access(
diff --git a/configmgr/source/access.cxx b/configmgr/source/access.cxx
index 41d86e7..b8fadbc 100644
--- a/configmgr/source/access.cxx
+++ b/configmgr/source/access.cxx
@@ -76,6 +76,7 @@
 #include cppuhelper/weak.hxx
 #include osl/interlck.h
 #include osl/mutex.hxx
+#include rtl/character.hxx
 #include rtl/ref.hxx
 #include rtl/ustrbuf.hxx
 #include rtl/ustring.h
@@ -103,6 +104,26 @@
 
 namespace configmgr {
 
+namespace {
+
+// Conservatively forbid what is either not an XML Char (including lone
+// surrogates, even though they should not appear in well-formed UNO OUString
+// instances anyway), or is a slash (as it causes problems in path syntax):
+bool isValidName(OUString const  name) {
+for (sal_Int32 i = 0; i != name.getLength();) {
+sal_uInt32 c = name.iterateCodePoints(i);
+if ((c  0x20  !(c == 0x09 || c == 0x0A || c == 0x0D))
+|| rtl::isHighSurrogate(c) || rtl::isLowSurrogate(c) || c == 0xFFFE
+|| c == 0x || c == '/')
+{
+return false;
+}
+}
+return !name.isEmpty();
+}
+
+}
+
 oslInterlockedCount Access::acquireCounting() {
 return osl_atomic_increment(m_refCount);
 }
@@ -648,6 +669,10 @@ void Access::setName(OUString const  aName)
 if (node-getMandatory() == Data::NO_LAYER 
 !(other.is()  other-isFinalized()))
 {
+if (!isValidName(aName)) {
+throw css::uno::RuntimeException(
+invalid element name  +