Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Merge pull request #3864 from elpaso/downloader-fixes-bad-network
[BUGFIX] Fixed a crash on bad network protocol
  • Loading branch information
elpaso committed Dec 13, 2016
2 parents 01d5233 + 9d9d162 commit f48f90e
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 20 deletions.
1 change: 0 additions & 1 deletion src/app/qgsidentifyresultsdialog.cpp
Expand Up @@ -77,7 +77,6 @@ QgsIdentifyResultsWebView::QgsIdentifyResultsWebView( QWidget *parent ) : QgsWeb
{
setSizePolicy( QSizePolicy::MinimumExpanding, QSizePolicy::Minimum );
page()->setNetworkAccessManager( QgsNetworkAccessManager::instance() );
// page()->setLinkDelegationPolicy( QWebPage::DelegateAllLinks );
#ifdef WITH_QTWEBKIT
page()->setForwardUnsupportedContent( true );
#endif
Expand Down
11 changes: 6 additions & 5 deletions src/gui/qgsfiledownloader.cpp
Expand Up @@ -60,7 +60,6 @@ void QgsFileDownloader::startDownload()
mReply = nam->get( request );

connect( mReply, &QNetworkReply::readyRead, this, &QgsFileDownloader::onReadyRead );
connect( mReply, static_cast < void ( QNetworkReply::* )( QNetworkReply::NetworkError ) > ( &QNetworkReply::error ), this, &QgsFileDownloader::onNetworkError );
connect( mReply, &QNetworkReply::finished, this, &QgsFileDownloader::onFinished );
connect( mReply, &QNetworkReply::downloadProgress, this, &QgsFileDownloader::onDownloadProgress );
connect( nam, &QgsNetworkAccessManager::requestTimedOut, this, &QgsFileDownloader::onRequestTimedOut );
Expand Down Expand Up @@ -108,7 +107,7 @@ void QgsFileDownloader::error( QStringList errorMessages )
{
for ( auto end = errorMessages.size(), i = 0; i != end; ++i )
{
mErrors.append( errorMessages[i] );
mErrors << errorMessages[i];
}
// Show error
if ( mGuiNotificationsEnabled )
Expand Down Expand Up @@ -161,7 +160,7 @@ void QgsFileDownloader::onFinished()
if ( mReply->error() )
{
mFile.remove();
error( tr( "Download failed: %1." ).arg( mReply->errorString() ) );
error( tr( "Download failed: %1" ).arg( mReply->errorString() ) );
}
else if ( !redirectionTarget.isNull() )
{
Expand All @@ -174,8 +173,10 @@ void QgsFileDownloader::onFinished()
startDownload();
return;
}
// All done
emit downloadCompleted();
else
{
emit downloadCompleted();
}
}
emit downloadExited();
this->deleteLater();
Expand Down
16 changes: 8 additions & 8 deletions tests/src/gui/testqgsfiledownloader.cpp
Expand Up @@ -78,8 +78,8 @@ class TestQgsFileDownloader: public QObject

void testValidDownload();
void testInValidDownload();
void testCanceledDownload();
void testInvalidFile();
void testCanceledDownload();
void testInvalidUrl();
void testBlankUrl();
#ifndef QT_NO_SSL
Expand Down Expand Up @@ -172,7 +172,7 @@ void TestQgsFileDownloader::testInValidDownload()
QVERIFY( mError );
QVERIFY( !mCanceled );
QVERIFY( mTempFile->size() == 0 );
QCOMPARE( mErrorMessage, QString( "Network error 3: Host www.doesnotexistofthatimsure.qgis not found" ) );
QCOMPARE( mErrorMessage, QString( "Download failed: Host www.doesnotexistofthatimsure.qgis not found" ) );
}

void TestQgsFileDownloader::testCanceledDownload()
Expand All @@ -184,7 +184,7 @@ void TestQgsFileDownloader::testCanceledDownload()
QVERIFY( !mError );
QVERIFY( mProgress );
QVERIFY( mCanceled );
QVERIFY( mTempFile->size() == 0 );
QVERIFY( !mTempFile->exists() );
}

void TestQgsFileDownloader::testInvalidFile()
Expand All @@ -205,7 +205,7 @@ void TestQgsFileDownloader::testInvalidUrl()
QVERIFY( !mCompleted );
QVERIFY( mError );
QVERIFY( !mCanceled );
QCOMPARE( mErrorMessage, QString( "Network error 301: Protocol \"xyz\" is unknown" ) );
QCOMPARE( mErrorMessage, QString( "Download failed: Protocol \"xyz\" is unknown" ) );
}

void TestQgsFileDownloader::testBlankUrl()
Expand All @@ -216,7 +216,7 @@ void TestQgsFileDownloader::testBlankUrl()
QVERIFY( !mCompleted );
QVERIFY( mError );
QVERIFY( !mCanceled );
QCOMPARE( mErrorMessage, QString( "Network error 301: Protocol \"\" is unknown" ) );
QCOMPARE( mErrorMessage, QString( "Download failed: Protocol \"\" is unknown" ) );
}

#ifndef QT_NO_SSL
Expand All @@ -226,11 +226,11 @@ void TestQgsFileDownloader::testSslError_data()
QTest::addColumn<QString>( "result" );

QTest::newRow( "expired" ) << "https://expired.badssl.com/"
<< "Network error 6: SSL handshake failed;SSL Errors: ;The certificate has expired";
<< "SSL Errors: ;The certificate has expired";
QTest::newRow( "self-signed" ) << "https://self-signed.badssl.com/"
<< "Network error 6: SSL handshake failed;SSL Errors: ;The certificate is self-signed, and untrusted";
<< "SSL Errors: ;The certificate is self-signed, and untrusted";
QTest::newRow( "untrusted-root" ) << "https://untrusted-root.badssl.com/"
<< "Network error 6: SSL handshake failed;No certificates could be verified;SSL Errors: ;The issuer certificate of a locally looked up certificate could not be found";
<< "No certificates could be verified;SSL Errors: ;The issuer certificate of a locally looked up certificate could not be found";
}

void TestQgsFileDownloader::testSslError()
Expand Down
12 changes: 6 additions & 6 deletions tests/src/python/test_qgsfiledownloader.py
Expand Up @@ -78,7 +78,7 @@ def test_inValidDownload(self):
self.assertTrue(self.progress_was_called)
self.assertFalse(self.canceled_was_called)
self.assertTrue(self.error_was_called)
self.assertEqual(self.error_args[1], [u'Network error 3: Host www.doesnotexistofthatimsure.qgis not found'])
self.assertEqual(self.error_args[1], [u'Download failed: Host www.doesnotexistofthatimsure.qgis not found'])
self.assertFalse(os.path.isfile(destination))

def test_dowloadCanceled(self):
Expand All @@ -99,7 +99,7 @@ def test_InvalidUrl(self):
self.assertFalse(self.canceled_was_called)
self.assertTrue(self.error_was_called)
self.assertFalse(os.path.isfile(destination))
self.assertEqual(self.error_args[1], [u"Network error 301: Protocol \"xyz\" is unknown"])
self.assertEqual(self.error_args[1], [u"Download failed: Protocol \"xyz\" is unknown"])

def test_InvalidFile(self):
self._make_download('https://github.com/qgis/QGIS/archive/master.zip', "")
Expand All @@ -117,7 +117,7 @@ def test_BlankUrl(self):
self.assertFalse(self.canceled_was_called)
self.assertTrue(self.error_was_called)
self.assertFalse(os.path.isfile(destination))
self.assertEqual(self.error_args[1], [u"Network error 301: Protocol \"\" is unknown"])
self.assertEqual(self.error_args[1], [u"Download failed: Protocol \"\" is unknown"])

def ssl_compare(self, name, url, error):
destination = tempfile.mktemp()
Expand All @@ -133,9 +133,9 @@ def ssl_compare(self, name, url, error):
self.assertTrue(result.startswith(error), msg + "expected:\n%s\nactual:\n%s\n" % (result, error))

def test_sslExpired(self):
self.ssl_compare("expired", "https://expired.badssl.com/", "Network error 6: SSL handshake failed;SSL Errors: ;The certificate has expired")
self.ssl_compare("self-signed", "https://self-signed.badssl.com/", "Network error 6: SSL handshake failed;SSL Errors: ;The certificate is self-signed, and untrusted")
self.ssl_compare("untrusted-root", "https://untrusted-root.badssl.com/", "Network error 6: SSL handshake failed;No certificates could be verified;")
self.ssl_compare("expired", "https://expired.badssl.com/", "SSL Errors: ;The certificate has expired")
self.ssl_compare("self-signed", "https://self-signed.badssl.com/", "SSL Errors: ;The certificate is self-signed, and untrusted")
self.ssl_compare("untrusted-root", "https://untrusted-root.badssl.com/", "No certificates could be verified;SSL Errors: ;The issuer certificate of a locally looked up certificate could not be found")

def _set_slot(self, *args, **kwargs):
#print('_set_slot(%s) called' % args[0])
Expand Down

0 comments on commit f48f90e

Please sign in to comment.