Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[bugfix] Crash when exporting (invalid) legend to qlr #7195

Merged
merged 5 commits into from
Jun 7, 2018
Merged

[bugfix] Crash when exporting (invalid) legend to qlr #7195

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ec909b0
[bugfix] Crash when exporting (invalid) legend to qlr
elpaso Jun 6, 2018
2f2221a
Do not return invalid layers from findLayers()
elpaso Jun 7, 2018
a389321
Include QDomNode (needed to build the test)
elpaso Jun 7, 2018
ad49cfb
Add test for QgsLayerDefinition (regression #18981)
elpaso Jun 7, 2018
b307f39
Added broken qlr file for the test case
elpaso Jun 7, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/core/layertree/qgslayertreegroup.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ QList<QgsLayerTreeLayer *> QgsLayerTreeGroup::findLayers() const
QList<QgsLayerTreeLayer *> list;
Q_FOREACH ( QgsLayerTreeNode *child, mChildren )
{
if ( QgsLayerTree::isLayer( child ) )
if ( QgsLayerTree::isLayer( child ) && QgsLayerTree::toLayer( child )->layer( ) )
list << QgsLayerTree::toLayer( child );
else if ( QgsLayerTree::isGroup( child ) )
list << QgsLayerTree::toGroup( child )->findLayers();
Expand Down
6 changes: 5 additions & 1 deletion src/core/qgslayerdefinition.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
* (at your option) any later version. *
* *
***************************************************************************/
#include <QDomNode>
#include <QFileInfo>
#include <QFile>
#include <QDir>
Expand Down Expand Up @@ -214,6 +213,11 @@ bool QgsLayerDefinition::exportLayerDefinition( QDomDocument doc, const QList<Qg
QList<QgsLayerTreeLayer *> layers = root->findLayers();
Q_FOREACH ( QgsLayerTreeLayer *layer, layers )
{
if ( ! layer->layer() )
{
QgsDebugMsgLevel( QStringLiteral( "Not a valid map layer: skipping %1" ).arg( layer->name( ) ), 4 );
continue;
}
QDomElement layerelm = doc.createElement( QStringLiteral( "maplayer" ) );
layer->layer()->writeLayerXml( layerelm, doc, context );
layerselm.appendChild( layerelm );
Expand Down
2 changes: 1 addition & 1 deletion src/core/qgslayerdefinition.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@

#include <QString>
#include <QVector>
#include <QDomNode>

class QDomNode;
class QDomDocument;

class QgsLayerTreeGroup;
Expand Down
1 change: 1 addition & 0 deletions tests/src/core/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ SET(TESTS
testziplayer.cpp
testqgsmeshlayer.cpp
testqgsmeshlayerrenderer.cpp
testqgslayerdefinition.cpp
)

IF(WITH_QTWEBKIT)
Expand Down
107 changes: 107 additions & 0 deletions tests/src/core/testqgslayerdefinition.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
/***************************************************************************
testqgsfilefiledownloader.cpp
--------------------------------------
Date : 07.06.2018
Copyright : (C) 2018 Alessandro Pasotti
Email : elpaso at itopen dot it
***************************************************************************
* *
* This program is free software; you can redistribute it and/or modify *
* it under the terms of the GNU General Public License as published by *
* the Free Software Foundation; either version 2 of the License, or *
* (at your option) any later version. *
* *
***************************************************************************/


#include "qgstest.h"
#include <QObject>
#include <QTemporaryFile>
#include <QTemporaryDir>

#include <qgsapplication.h>
#include <qgsproject.h>
#include <qgslayertree.h>
#include <qgslayerdefinition.h>

class TestQgsLayerDefinition: public QObject
{
Q_OBJECT
public:
TestQgsLayerDefinition() = default;

private slots:
void initTestCase(); // will be called before the first testfunction is executed.
void cleanupTestCase(); // will be called after the last testfunction was executed.
void init(); // will be called before each testfunction is executed.
void cleanup(); // will be called after every testfunction.

/**
* test that findLayers() skips invalid layers
*/
void testFindLayers();

/**
* test that export does not crash: regression #18981
* https://issues.qgis.org/issues/18981 - Save QLR crashes QGIS 3
*/
void testExportDoesNotCrash();

private:
QTemporaryFile *mTempFile;
};


void TestQgsLayerDefinition::initTestCase()
{
QgsApplication::init();
QgsApplication::initQgis();

}

void TestQgsLayerDefinition::cleanupTestCase()
{
QgsApplication::exitQgis();
}

void TestQgsLayerDefinition::init()
{
mTempFile = new QTemporaryFile();
QVERIFY( mTempFile->open() );
mTempFile->close();
QString errorMessage;
const QString path = QString( TEST_DATA_DIR + QStringLiteral( "/bug_18981_broken.qlr" ) );
QgsLayerDefinition::loadLayerDefinition( path, QgsProject::instance(), QgsProject::instance()->layerTreeRoot(), errorMessage );
QVERIFY( errorMessage.isEmpty() );
}


void TestQgsLayerDefinition::cleanup()
{
delete mTempFile;
}

void TestQgsLayerDefinition::testFindLayers()
{
QCOMPARE( QgsProject::instance()->layerTreeRoot()->findLayers().count(), 1 );
QCOMPARE( QgsProject::instance()->layerTreeRoot()->findLayers().at( 0 )->name(), QStringLiteral( "NewMemory" ) );
}

void TestQgsLayerDefinition::testExportDoesNotCrash()
{
QString errorMessage;
QVERIFY( QgsLayerDefinition::exportLayerDefinition( mTempFile->fileName(), QgsProject::instance()->layerTreeRoot()->children(), errorMessage ) );
QVERIFY( errorMessage.isEmpty() );
// Reload
const QString path = QString( TEST_DATA_DIR + QStringLiteral( "/bug_18981_broken.qlr" ) );
QgsProject::instance()->removeAllMapLayers();
QgsLayerDefinition::loadLayerDefinition( path, QgsProject::instance(), QgsProject::instance()->layerTreeRoot(), errorMessage );
testFindLayers();
}



QGSTEST_MAIN( TestQgsLayerDefinition )
#include "testqgslayerdefinition.moc"


150 changes: 150 additions & 0 deletions tests/testdata/bug_18981_broken.qlr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
<!DOCTYPE qgis-layer-definition>
<qlr>
<layer-tree-group expanded="1" checked="Qt::Checked" name="">
<customproperties/>
<layer-tree-group expanded="1" checked="Qt::Checked" name="qgis_plugin">
<customproperties/>
<layer-tree-group expanded="0" checked="Qt::Checked" name="Baggrundskort">
<customproperties/>
<layer-tree-layer expanded="1" providerKey="wms" checked="Qt::Checked" id="DTK_D8502016120170201702017020170201702017201801241314424672312" source="IgnoreGetMapUrl=1&amp;contextualWMSLegend=0&amp;crs=EPSG:25832&amp;dpiMode=7&amp;featureCount=10&amp;format=image/png&amp;layers=dtk_d850&amp;styles=&amp;url=http://kortforsyningen.kms.dk/service?servicename%3Dtopo_basis%26client%3DQGIS%26version%3D1.1.1%26login%3Dj%26password%3Dj" name="DTK/D850">
<customproperties/>
</layer-tree-layer>
<layer-tree-layer providerKey="memory" checked="Qt::Checked" expanded="1" name="NewMemory" source="NoGeometry?crs=EPSG:4326&amp;uid={64c9bcbf-cabe-4bd9-9058-2b80ebf87a72}" id="NewMemory_ffa4d8a4_e5be_46a9_a0e0_fb6ee924f3cd">
<customproperties/>
</layer-tree-layer>
</layer-tree-group>
</layer-tree-group>
</layer-tree-group>
<maplayers>
<maplayer minimumScale="0" maximumScale="1e+08" type="raster" hasScaleBasedVisibilityFlag="0">
<extent>
<xmin>120000</xmin>
<ymin>5900000</ymin>
<xmax>1000000</xmax>
<ymax>6500000</ymax>
</extent>
<id>DTK_D8502016120170201702017020170201702017201801241314424672312</id>
<datasource>IgnoreGetMapUrl=1&amp;contextualWMSLegend=0&amp;crs=EPSG:25832&amp;dpiMode=7&amp;featureCount=10&amp;format=image/png&amp;layers=dtk_d850&amp;styles=&amp;url=http://kortforsyningen.kms.dk/service?servicename%3Dtopo_basis%26client%3DQGIS%26version%3D1.1.1%26login%3Dj%26password%3Dj</datasource>
<keywordList>
<value></value>
</keywordList>
<layername>DTK/D850</layername>
<srs>
<spatialrefsys>
<proj4>+proj=utm +zone=32 +ellps=GRS80 +towgs84=0,0,0,0,0,0,0 +units=m +no_defs</proj4>
<srsid>2105</srsid>
<srid>25832</srid>
<authid>EPSG:25832</authid>
<description>ETRS89 / UTM zone 32N</description>
<projectionacronym>utm</projectionacronym>
<ellipsoidacronym>GRS80</ellipsoidacronym>
<geographicflag>false</geographicflag>
</spatialrefsys>
</srs>
<customproperties>
<property key="identify/format" value="Undefined"/>
</customproperties>
<provider>wms</provider>
<noData>
<noDataList bandNo="1" useSrcNoData="0"/>
</noData>
<map-layer-style-manager current="">
<map-layer-style name=""/>
</map-layer-style-manager>
<pipe>
<rasterrenderer opacity="1" alphaBand="-1" band="1" type="singlebandcolordata">
<rasterTransparency/>
</rasterrenderer>
<brightnesscontrast brightness="0" contrast="0"/>
<huesaturation colorizeGreen="128" colorizeOn="0" colorizeRed="255" colorizeBlue="128" grayscaleMode="0" saturation="0" colorizeStrength="100"/>
<rasterresampler maxOversampling="2"/>
</pipe>
<blendMode>0</blendMode>
</maplayer>
<maplayer refreshOnNotifyEnabled="0" readOnly="0" minScale="1e+8" type="vector" autoRefreshTime="0" hasScaleBasedVisibilityFlag="0" geometry="No geometry" autoRefreshEnabled="0" refreshOnNotifyMessage="" maxScale="0">
<id>NewMemory_ffa4d8a4_e5be_46a9_a0e0_fb6ee924f3cd</id>
<datasource>memory?geometry=NoGeometry&amp;crs=EPSG:4326</datasource>
<keywordList>
<value></value>
</keywordList>
<layername>NewMemory</layername>
<srs>
<spatialrefsys>
<proj4>+proj=longlat +datum=WGS84 +no_defs</proj4>
<srsid>3452</srsid>
<srid>4326</srid>
<authid>EPSG:4326</authid>
<description>WGS 84</description>
<projectionacronym>longlat</projectionacronym>
<ellipsoidacronym>WGS84</ellipsoidacronym>
<geographicflag>true</geographicflag>
</spatialrefsys>
</srs>
<resourceMetadata>
<identifier></identifier>
<parentidentifier></parentidentifier>
<language></language>
<type></type>
<title></title>
<abstract></abstract>
<links/>
<fees></fees>
<encoding></encoding>
<crs>
<spatialrefsys>
<proj4></proj4>
<srsid>0</srsid>
<srid>0</srid>
<authid></authid>
<description></description>
<projectionacronym></projectionacronym>
<ellipsoidacronym></ellipsoidacronym>
<geographicflag>false</geographicflag>
</spatialrefsys>
</crs>
<extent/>
</resourceMetadata>
<customproperties/>
<provider encoding="UTF-8">memory</provider>
<vectorjoins/>
<layerDependencies/>
<dataDependencies/>
<legend type="default-vector"/>
<expressionfields/>
<map-layer-style-manager current="predefinito">
<map-layer-style name="predefinito"/>
</map-layer-style-manager>
<auxiliaryLayer/>
<fieldConfiguration/>
<aliases/>
<excludeAttributesWMS/>
<excludeAttributesWFS/>
<defaults/>
<constraints/>
<constraintExpressions/>
<attributeactions>
<defaultAction key="Canvas" value="{00000000-0000-0000-0000-000000000000}"/>
</attributeactions>
<attributetableconfig sortOrder="0" actionWidgetStyle="dropDown" sortExpression="">
<columns/>
</attributetableconfig>
<editform></editform>
<editforminit/>
<editforminitcodesource>0</editforminitcodesource>
<editforminitfilepath></editforminitfilepath>
<editforminitcode><![CDATA[]]></editforminitcode>
<featformsuppress>0</featformsuppress>
<editorlayout>generatedlayout</editorlayout>
<editable/>
<labelOnTop/>
<widgets/>
<conditionalstyles>
<rowstyles/>
<fieldstyles/>
</conditionalstyles>
<expressionfields/>
<previewExpression></previewExpression>
<mapTip></mapTip>
</maplayer>
</maplayers>
</qlr>