Skip to content

Commit

Permalink
[BACKPORT][BUGFIX] Fixed a crash on bad network protocol
Browse files Browse the repository at this point in the history
 ( from f48f90e )
  • Loading branch information
elpaso committed Dec 14, 2016
1 parent d44f1eb commit a848b8c
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 24 deletions.
14 changes: 5 additions & 9 deletions src/gui/qgsfiledownloader.cpp
Expand Up @@ -60,7 +60,6 @@ void QgsFileDownloader::startDownload()
mReply = nam->get( request );

connect( mReply, SIGNAL( readyRead() ), this, SLOT( onReadyRead() ) );
connect( mReply, SIGNAL( error( QNetworkReply::NetworkError ) ), this, SLOT( onNetworkError( QNetworkReply::NetworkError ) ) );
connect( mReply, SIGNAL( finished() ), this, SLOT( onFinished() ) );
connect( mReply, SIGNAL( downloadProgress( qint64, qint64 ) ), this, SLOT( onDownloadProgress( qint64, qint64 ) ) );
connect( nam, SIGNAL( requestTimedOut( QNetworkReply* ) ), this, SLOT( onRequestTimedOut() ) );
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,18 +173,15 @@ void QgsFileDownloader::onFinished()
startDownload();
return;
}
// All done
emit downloadCompleted();
else
{
emit downloadCompleted();
}
}
emit downloadExited();
this->deleteLater();
}

void QgsFileDownloader::onNetworkError( QNetworkReply::NetworkError err )
{
Q_ASSERT( mReply );
error( QString( "Network error %1: %2" ).arg( err ).arg( mReply->errorString() ) );
}

void QgsFileDownloader::onDownloadProgress( qint64 bytesReceived, qint64 bytesTotal )
{
Expand Down
2 changes: 0 additions & 2 deletions src/gui/qgsfiledownloader.h
Expand Up @@ -75,8 +75,6 @@ class GUI_EXPORT QgsFileDownloader : public QObject
void onReadyRead();
/** Called when the network reply has finished */
void onFinished();
/** Called on Network Error */
void onNetworkError( QNetworkReply::NetworkError err );
/** Called on data ready to be processed */
void onDownloadProgress( qint64 bytesReceived, qint64 bytesTotal );
/** Called when a network request times out */
Expand Down
12 changes: 6 additions & 6 deletions tests/src/gui/testqgsfiledownloader.cpp
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 Down Expand Up @@ -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_OPENSSL
Expand All @@ -225,9 +225,9 @@ void TestQgsFileDownloader::testSslError_data()
QTest::addColumn<QString>( "url" );
QTest::addColumn<QString>( "result" );

QTest::newRow( "expired" ) << "https://expired.badssl.com/" << "Network error 6: SSL handshake failed;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";
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;The root CA certificate is not trusted for this purpose";
QTest::newRow( "expired" ) << "https://expired.badssl.com/" << "SSL Errors: ;The certificate has expired";
QTest::newRow( "self-signed" ) << "https://self-signed.badssl.com/" << "SSL Errors: ;The certificate is self-signed, and untrusted";
QTest::newRow( "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;The root CA certificate is not trusted for this purpose";
}

void TestQgsFileDownloader::testSslError()
Expand Down
14 changes: 7 additions & 7 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 @@ -130,12 +130,12 @@ def ssl_compare(self, name, url, error):
self.assertFalse(os.path.isfile(destination), msg)
result = sorted(self.error_args[1])
result = ';'.join(result)
self.assertEqual(result, error, msg + "expected:\n%s\nactual:\n%s\n" % (result, error))
self.assertEqual(result, error, msg + "expected:\n%s\nactual:\n%s\n" % (error, result))

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;SSL Errors: ;The issuer certificate of a locally looked up certificate could not be found;The root CA certificate is not trusted for this purpose")
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;The root CA certificate is not trusted for this purpose")

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

0 comments on commit a848b8c

Please sign in to comment.