Skip to content

Commit

Permalink
Allow free naming of project properties
Browse files Browse the repository at this point in the history 
Allows:

<properties name="properties">
  <properties name="1">
    <properties name="2" type="QString">foo</properties>
  </properties>
</properties>

Data-driven properties names could violate that monstrous regular
expression earlier (eg. using layer ids with leading digits as key was
illegal) and trigger "Entry token invalid" for no too obvious reason
(even when trying to remove a non-existing entry).

Turns

<properties>
  <foo>
    <bar type="QString">baz</bar>
  </foo>
</properties>

into

<properties name="properties">
  <properties name="foo">
    <properties name="bar" type="QString">baz</properties>
  </properties>
</properties>
  • Loading branch information
@jef-n
jef-n committed Mar 7, 2025
1 parent 9a1d453 commit 8f84b3c
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 126 deletions.
58 changes: 29 additions & 29 deletions src/core/project/qgsproject.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,21 +116,6 @@ QStringList makeKeyTokens_( const QString &scope, const QString &key )
// be sure to include the canonical root node
keyTokens.push_front( QStringLiteral( "properties" ) );

//check validy of keys since an invalid xml name will will be dropped upon saving the xml file. If not valid, we print a message to the console.
for ( int i = 0; i < keyTokens.size(); ++i )
{
const QString keyToken = keyTokens.at( i );

//invalid chars in XML are found at http://www.w3.org/TR/REC-xml/#NT-NameChar
//note : it seems \x10000-\xEFFFF is valid, but it when added to the regexp, a lot of unwanted characters remain
const thread_local QRegularExpression sInvalidRegexp = QRegularExpression( QStringLiteral( "([^:A-Z_a-z\\x{C0}-\\x{D6}\\x{D8}-\\x{F6}\\x{F8}-\\x{2FF}\\x{370}-\\x{37D}\\x{37F}-\\x{1FFF}\\x{200C}-\\x{200D}\\x{2070}-\\x{218F}\\x{2C00}-\\x{2FEF}\\x{3001}-\\x{D7FF}\\x{F900}-\\x{FDCF}\\x{FDF0}-\\x{FFFD}\\-\\.0-9\\x{B7}\\x{0300}-\\x{036F}\\x{203F}-\\x{2040}]|^[^:A-Z_a-z\\x{C0}-\\x{D6}\\x{D8}-\\x{F6}\\x{F8}-\\x{2FF}\\x{370}-\\x{37D}\\x{37F}-\\x{1FFF}\\x{200C}-\\x{200D}\\x{2070}-\\x{218F}\\x{2C00}-\\x{2FEF}\\x{3001}-\\x{D7FF}\\x{F900}-\\x{FDCF}\\x{FDF0}-\\x{FFFD}])" ) );
if ( keyToken.contains( sInvalidRegexp ) )
{
const QString errorString = QObject::tr( "Entry token invalid : '%1'. The token will not be saved to file." ).arg( keyToken );
QgsMessageLog::logMessage( errorString, QString(), Qgis::MessageLevel::Critical );
}
}

return keyTokens;
}

Expand Down Expand Up @@ -1323,20 +1308,20 @@ void dump_( const QgsProjectPropertyKey &topQgsPropertyKey )
* scope. "layers" is a list containing three string values.
*
* \code{.xml}
* <properties>
* <fsplugin>
* <foo type="int" >42</foo>
* <baz type="int" >1</baz>
* <layers type="QStringList" >
* <properties name="properties">
* <properties name="fsplugin">
* <properties name="foo" type="int" >42</properties>
* <properties name="baz" type="int" >1</properties>
* <properties name="layers" type="QStringList">
* <value>railroad</value>
* <value>airport</value>
* </layers>
* <xyqzzy type="int" >1</xyqzzy>
* <bar type="double" >123.456</bar>
* <feature_types type="QStringList" >
* </properties>
* <properties name="xyqzzy" type="int" >1</properties>
* <properties name="bar" type="double" >123.456</properties>
* <properties name="feature_types" type="QStringList">
* <value>type</value>
* </feature_types>
* </fsplugin>
* </properties>
* </properties>
* </properties>
* \endcode
*
Expand Down Expand Up @@ -3984,10 +3969,25 @@ bool QgsProject::createEmbeddedLayer( const QString &layerId, const QString &pro
const QDomElement propertiesElem = sProjectDocument.documentElement().firstChildElement( QStringLiteral( "properties" ) );
if ( !propertiesElem.isNull() )
{
const QDomElement absElem = propertiesElem.firstChildElement( QStringLiteral( "Paths" ) ).firstChildElement( QStringLiteral( "Absolute" ) );
if ( !absElem.isNull() )
QDomElement e = propertiesElem.firstChildElement( QStringLiteral( "Paths" ) );
if ( e.isNull() )
{
e = propertiesElem.firstChildElement( QStringLiteral( "properties" ) );
while ( !e.isNull() && e.attribute( QStringLiteral( "name" ) ) != QStringLiteral( "Paths" ) )
e = e.nextSiblingElement( QStringLiteral( "properties" ) );

e = e.firstChildElement( QStringLiteral( "properties" ) );
while ( !e.isNull() && e.attribute( QStringLiteral( "name" ) ) != QStringLiteral( "Absolute" ) )
e = e.nextSiblingElement( QStringLiteral( "properties" ) );
}
else
{
e = e.firstChildElement( QStringLiteral( "Absolute" ) );
}

if ( !e.isNull() )
{
useAbsolutePaths = absElem.text().compare( QLatin1String( "true" ), Qt::CaseInsensitive ) == 0;
useAbsolutePaths = e.text().compare( QLatin1String( "true" ), Qt::CaseInsensitive ) == 0;
}
}

Expand Down
47 changes: 28 additions & 19 deletions src/core/project/qgsprojectproperty.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -233,15 +233,15 @@ bool QgsProjectPropertyValue::readXml( const QDomNode &keyNode )

// keyElement is created by parent QgsProjectPropertyKey
bool QgsProjectPropertyValue::writeXml( QString const &nodeName,
QDomElement &keyElement,
QDomDocument &document )
QDomElement &keyElement,
QDomDocument &document )
{
QDomElement valueElement = document.createElement( nodeName );
QDomElement valueElement = document.createElement( QStringLiteral( "properties" ) );

// remember the type so that we can rebuild it when the project is read in
valueElement.setAttribute( QStringLiteral( "name" ), nodeName );
valueElement.setAttribute( QStringLiteral( "type" ), mValue.typeName() );


// we handle string lists differently from other types in that we
// create a sequence of repeated elements to cover all the string list
// members; each value will be in a <value></value> tag.
Expand Down Expand Up @@ -362,33 +362,41 @@ bool QgsProjectPropertyKey::readXml( const QDomNode &keyNode )

while ( i < subkeys.count() )
{
const QDomNode subkey = subkeys.item( i );
QString name;

if ( subkey.nodeName() == QStringLiteral( "properties" ) &&
subkey.hasAttributes() && // if we have attributes
subkey.isElement() && // and we're an element
subkey.toElement().hasAttribute( QStringLiteral( "name" ) ) ) // and we have a "name" attribute
name = subkey.toElement().attribute( QStringLiteral( "name" ) );
else
name = subkey.nodeName();

// if the current node is an element that has a "type" attribute,
// then we know it's a leaf node; i.e., a subkey _value_, and not
// a subkey
if ( subkeys.item( i ).hasAttributes() && // if we have attributes
subkeys.item( i ).isElement() && // and we're an element
subkeys.item( i ).toElement().hasAttribute( QStringLiteral( "type" ) ) ) // and we have a "type" attribute
if ( subkey.hasAttributes() && // if we have attributes
subkey.isElement() && // and we're an element
subkey.toElement().hasAttribute( QStringLiteral( "type" ) ) ) // and we have a "type" attribute
{
// then we're a key value
delete mProperties.take( subkeys.item( i ).nodeName() );
mProperties.insert( subkeys.item( i ).nodeName(), new QgsProjectPropertyValue );
//
delete mProperties.take( name );
mProperties.insert( name, new QgsProjectPropertyValue );

QDomNode subkey = subkeys.item( i );

if ( !mProperties[subkeys.item( i ).nodeName()]->readXml( subkey ) )
if ( !mProperties[name]->readXml( subkey ) )
{
QgsDebugError( QStringLiteral( "unable to parse key value %1" ).arg( subkeys.item( i ).nodeName() ) );
QgsDebugError( QStringLiteral( "unable to parse key value %1" ).arg( name ) );
}
}
else // otherwise it's a subkey, so just recurse on down the remaining keys
{
addKey( subkeys.item( i ).nodeName() );

QDomNode subkey = subkeys.item( i );
addKey( name );

if ( !mProperties[subkeys.item( i ).nodeName()]->readXml( subkey ) )
if ( !mProperties[name]->readXml( subkey ) )
{
QgsDebugError( QStringLiteral( "unable to parse subkey %1" ).arg( subkeys.item( i ).nodeName() ) );
QgsDebugError( QStringLiteral( "unable to parse subkey %1" ).arg( name ) );
}
}

Expand All @@ -408,7 +416,8 @@ bool QgsProjectPropertyKey::writeXml( QString const &nodeName, QDomElement &elem
// If it's an _empty_ node (i.e., one with no properties) we need to emit
// an empty place holder; else create new Dom elements as necessary.

QDomElement keyElement = document.createElement( nodeName ); // Dom element for this property key
QDomElement keyElement = document.createElement( "properties" ); // Dom element for this property key
keyElement.toElement().setAttribute( QStringLiteral( "name" ), nodeName );

if ( ! mProperties.isEmpty() )
{
Expand Down
78 changes: 0 additions & 78 deletions tests/src/python/test_qgsproject.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,84 +63,6 @@ def __init__(self, methodName):
QgisTestCase.__init__(self, methodName)
self.messageCaught = False

def test_makeKeyTokens_(self):
# see http://www.w3.org/TR/REC-xml/#d0e804 for a list of valid characters

invalidTokens = []
validTokens = []

# all test tokens will be generated by prepending or inserting characters to this token
validBase = "valid"

# some invalid characters, not allowed anywhere in a token
# note that '/' must not be added here because it is taken as a separator by makeKeyTokens_()
invalidChars = "+*,;<>|!$%()=?#\x01"

# generate the characters that are allowed at the start of a token (and at every other position)
validStartChars = ":_"
charRanges = [
(ord("a"), ord("z")),
(ord("A"), ord("Z")),
(0x00F8, 0x02FF),
(0x0370, 0x037D),
(0x037F, 0x1FFF),
(0x200C, 0x200D),
(0x2070, 0x218F),
(0x2C00, 0x2FEF),
(0x3001, 0xD7FF),
(0xF900, 0xFDCF),
(0xFDF0, 0xFFFD),
# (0x10000, 0xEFFFF), while actually valid, these are not yet accepted by makeKeyTokens_()
]
for r in charRanges:
for c in range(r[0], r[1]):
validStartChars += chr(c)

# generate the characters that are only allowed inside a token, not at the start
validInlineChars = "-.\xB7"
charRanges = [
(ord("0"), ord("9")),
(0x0300, 0x036F),
(0x203F, 0x2040),
]
for r in charRanges:
for c in range(r[0], r[1]):
validInlineChars += chr(c)

# test forbidden start characters
for c in invalidChars + validInlineChars:
invalidTokens.append(c + validBase)

# test forbidden inline characters
for c in invalidChars:
invalidTokens.append(validBase[:4] + c + validBase[4:])

# test each allowed start character
for c in validStartChars:
validTokens.append(c + validBase)

# test each allowed inline character
for c in validInlineChars:
validTokens.append(validBase[:4] + c + validBase[4:])

logger = QgsApplication.messageLog()
logger.messageReceived.connect(self.catchMessage)
prj = QgsProject.instance()

for token in validTokens:
self.messageCaught = False
prj.readEntry("test", token)
myMessage = f"valid token '{token}' not accepted"
assert not self.messageCaught, myMessage

for token in invalidTokens:
self.messageCaught = False
prj.readEntry("test", token)
myMessage = f"invalid token '{token}' accepted"
assert self.messageCaught, myMessage

logger.messageReceived.disconnect(self.catchMessage)

def catchMessage(self):
self.messageCaught = True

Expand Down

0 comments on commit 8f84b3c

Please sign in to comment.