Merge branch 'fixup-log-upload' into calamares

CHANGES

@@ -15,10 +15,9 @@ This release contains contributions from (alphabetically by first name):
 
 ## Core ##
  - Uploading your log files (in case of installation failure) has been
-   expanded and is now more
-   configurable. Users should still take care when uploading logs,
-   and distro's should configure a URL with no public viewing
-   for the uploading of those logs. (Thanks Anubhav)
+   expanded and is now more configurable. Users should still take care
+   when uploading logs, and distro's should configure a URL with
+   no public viewing of those logs. (Thanks Anubhav)
 
 ## Modules ##
  - A new QML-based *finishedq* module has been added. (Thanks Anke)

src/branding/default/branding.desc

@@ -220,15 +220,13 @@ slideshowAPI: 2
 
 
 # These options are to customize online uploading of logs to pastebins:
-#  - type   :  Defines the kind of pastebin service to be used.Currently
-#              it accepts two values:
-#              - none    :    disables the pastebin functionality
-#              - fiche   :    use fiche pastebin server
-#  - url    :  Defines the address of pastebin service to be used.
-#              Takes string as input
-#  - port   :  Defines the port number to be used to send logs. Takes 
-#              integer as input
+#  - type : Defines the kind of pastebin service to be used. Currently
+#           it accepts two values:
+#           - none    :    disables the pastebin functionality
+#           - fiche   :    use fiche pastebin server
+#  - url  : Defines the address of pastebin service to be used.
+#           Takes string as input. Important bits are the host and port,
+#           the scheme is not used.
 uploadServer :
     type :    "fiche"
-    url :      "termbin.com"
-    port :     9999
+    url :     "http://termbin.com:9999"

src/libcalamaresui/Branding.cpp

@@ -138,6 +138,32 @@ loadStrings( QMap< QString, QString >& map,
     }
 }
 
+static Branding::UploadServerInfo
+uploadServerFromMap( const QVariantMap& map )
+{
+    using Type = Branding::UploadServerType;
+    // *INDENT-OFF*
+    // clang-format off
+    static const NamedEnumTable< Type > names {
+        { "none", Type::None },
+        { "fiche", Type::Fiche }
+    };
+    // clang-format on
+    // *INDENT-ON*
+
+    QString typestring = map[ "type" ].toString();
+    QString urlstring = map[ "url" ].toString();
+
+    if ( typestring.isEmpty() || urlstring.isEmpty() )
+    {
+        return Branding::UploadServerInfo( Branding::UploadServerType::None, QUrl() );
+    }
+
+    bool bogus = false;  // we don't care about type-name lookup success here
+    return Branding::UploadServerInfo( names.find( typestring, bogus ),
+                                       QUrl( urlstring, QUrl::ParsingMode::StrictMode ) );
+}
+
 /** @brief Load the @p map with strings from @p config
  *
  * If os-release is supported (with KF5 CoreAddons >= 5.58) then
@@ -227,11 +253,7 @@ Branding::Branding( const QString& brandingFilePath, QObject* parent )
             } );
             loadStrings( m_style, doc, "style", []( const QString& s ) -> QString { return s; } );
 
-            const QVariantMap temp = CalamaresUtils::yamlMapToVariant( doc[ "uploadServer" ] );
-            for ( auto it = temp.constBegin(); it != temp.constEnd(); ++it )
-            {
-                m_uploadServer.insert( it.key(), it.value().toString() );
-            }
+            m_uploadServer = uploadServerFromMap( CalamaresUtils::yamlMapToVariant( doc[ "uploadServer" ] ) );
         }
         catch ( YAML::Exception& e )
         {
@@ -292,12 +314,6 @@ Branding::imagePath( Branding::ImageEntry imageEntry ) const
     return m_images.value( s_imageEntryStrings.value( imageEntry ) );
 }
 
-QString
-Branding::uploadServer( Branding::UploadServerEntry uploadServerEntry ) const
-{
-    return m_uploadServer.value( s_uploadServerStrings.value( uploadServerEntry ) );
-}
-
 QPixmap
 Branding::image( Branding::ImageEntry imageEntry, const QSize& size ) const
 {

src/libcalamaresui/Branding.h

@@ -83,13 +83,16 @@ public:
     };
     Q_ENUM( StyleEntry )
 
-    enum UploadServerEntry : short
+    /** @brief Supported log-upload servers.
+     *
+     * 'None' is here as a fallback.
+     */
+    enum UploadServerType : short
     {
-        Type,
-        URL,
-        Port
+        None,
+        Fiche
     };
-    Q_ENUM( UploadServerEntry )
+    Q_ENUM( UploadServerType )
 
     /** @brief Setting for how much the main window may expand. */
     enum class WindowExpansion
@@ -218,6 +221,14 @@ public:
     ///@brief Which navigation flavor is configured
     PanelFlavor navigationFlavor() const { return m_navigationFlavor; }
 
+    /** @brief Upload server configuration
+     *
+     * This is both the type (which may be none, in which case the URL
+     * is irrelevant and usually empty) and the URL for the upload.
+     */
+    using UploadServerInfo = QPair< UploadServerType, QUrl >;
+    UploadServerInfo uploadServer() const { return m_uploadServer; }
+
     /**
      * Creates a map called "branding" in the global storage, and inserts an
      * entry for each of the branding strings. This makes the branding
@@ -234,7 +245,6 @@ public slots:
 
     QString styleString( StyleEntry styleEntry ) const;
     QString imagePath( ImageEntry imageEntry ) const;
-    QString uploadServer( UploadServerEntry uploadServerEntry ) const;
 
     PanelSide sidebarSide() const { return m_sidebarSide; }
     PanelSide navigationSide() const { return m_navigationSide; }
@@ -252,7 +262,7 @@ private:
     QMap< QString, QString > m_strings;
     QMap< QString, QString > m_images;
     QMap< QString, QString > m_style;
-    QMap< QString, QString > m_uploadServer;
+    UploadServerInfo m_uploadServer;
 
     /* The slideshow can be done in one of two ways:
      *  - as a sequence of images

src/libcalamaresui/CMakeLists.txt

@@ -111,3 +111,12 @@ foreach( subdir modulesystem utils viewpages widgets )
     file( GLOB subdir_headers "${subdir}/*.h" )
     install( FILES ${subdir_headers} DESTINATION include/libcalamares/${subdir} )
 endforeach()
+
+calamares_add_test(
+    test_libcalamaresuipaste
+    SOURCES
+        utils/TestPaste.cpp
+        utils/Paste.cpp
+    LIBRARIES
+        calamaresui
+)

src/libcalamaresui/ViewManager.cpp

@@ -27,6 +27,7 @@
 
 #include <QApplication>
 #include <QBoxLayout>
+#include <QClipboard>
 #include <QFile>
 #include <QMessageBox>
 #include <QMetaObject>
@@ -142,8 +143,8 @@ ViewManager::insertViewStep( int before, ViewStep* step )
 void
 ViewManager::onInstallationFailed( const QString& message, const QString& details )
 {
-    QString serverType = Calamares::Branding::instance()->uploadServer( Calamares::Branding::Type );
-    bool shouldOfferWebPaste = CalamaresUtils::UploadServersList.contains( serverType );
+    bool shouldOfferWebPaste
+        = Calamares::Branding::instance()->uploadServer().first != Calamares::Branding::UploadServerType::None;
 
     cError() << "Installation failed:" << message;
     cDebug() << Logger::SubEntry << "- message:" << message;
@@ -189,25 +190,26 @@ ViewManager::onInstallationFailed( const QString& message, const QString& detail
     connect( msgBox, &QMessageBox::buttonClicked, [msgBox]( QAbstractButton* button ) {
         if ( msgBox->buttonRole( button ) == QMessageBox::ButtonRole::YesRole )
         {
-            QString pasteUrlMsg;
-            QString serverType = Calamares::Branding::instance()->uploadServer( Calamares::Branding::Type );
-            if ( serverType == "fiche" )
+            QString pasteUrl = CalamaresUtils::Paste::doLogUpload( msgBox );
+            QString pasteUrlMessage;
+            if ( pasteUrl.isEmpty() )
             {
-                pasteUrlMsg = CalamaresUtils::ficheLogUpload( msgBox );
+                pasteUrlMessage = tr( "The upload was unsuccessful. No web-paste was done." );
             }
             else
             {
-                pasteUrlMsg = QString();
+                QClipboard* clipboard = QApplication::clipboard();
+                clipboard->setText( pasteUrl, QClipboard::Clipboard );
+
+                if ( clipboard->supportsSelection() )
+                {
+                    clipboard->setText( pasteUrl, QClipboard::Selection );
+                }
+                QString pasteUrlFmt = tr( "Install log posted to\n\n%1\n\nLink copied to clipboard" );
+                pasteUrlMessage = pasteUrlFmt.arg( pasteUrl );
             }
 
-            QString pasteUrlTitle = tr( "Install Log Paste URL" );
-            if ( pasteUrlMsg.isEmpty() )
-            {
-                pasteUrlMsg = tr( "The upload was unsuccessful. No web-paste was done." );
-            }
-
-            // TODO: make the URL clickable, or copy it to the clipboard automatically
-            QMessageBox::critical( nullptr, pasteUrlTitle, pasteUrlMsg );
+            QMessageBox::critical( nullptr, tr( "Install Log Paste URL" ), pasteUrlMessage );
         }
         QApplication::quit();
     } );

src/libcalamaresui/utils/Paste.cpp

@@ -10,49 +10,36 @@
 #include "Paste.h"
 
 #include "Branding.h"
+#include "DllMacro.h"
 #include "utils/Logger.h"
 
 #include <QFile>
-#include <QRegularExpression>
 #include <QTcpSocket>
 #include <QUrl>
-#include <QClipboard>
-#include <QApplication>
-#include <QStringList>
 
-namespace CalamaresUtils
+/** @brief Reads the logfile, returns its contents.
+ *
+ * Returns an empty QByteArray() on any kind of error.
+ */
+STATICTEST QByteArray
+logFileContents()
 {
-
-QStringList UploadServersList = {
-    "fiche"
-    // In future more serverTypes can be added as Calamares support them
-    // "none" serverType is explicitly not mentioned here
-};
-
-QString
-ficheLogUpload( QObject* parent )
-{
-
-    const QString& ficheHost = Calamares::Branding::instance()->uploadServer( Calamares::Branding::URL );
-    quint16 fichePort = Calamares::Branding::instance()->uploadServer( Calamares::Branding::Port ).toInt();
-
-    QString pasteUrlFmt = parent->tr( "Install log posted to\n\n%1\n\nLink copied to clipboard" );
-
     QFile pasteSourceFile( Logger::logFile() );
     if ( !pasteSourceFile.open( QIODevice::ReadOnly | QIODevice::Text ) )
     {
         cError() << "Could not open log file";
-        return QString();
+        return QByteArray();
     }
+    // TODO: read the **last** 16kiB?
+    return pasteSourceFile.read( 16384 /* bytes */ );
+}
 
-    QByteArray pasteData;
-    while ( !pasteSourceFile.atEnd() )
-    {
-        pasteData += pasteSourceFile.readLine();
-    }
 
+STATICTEST QString
+ficheLogUpload( const QByteArray& pasteData, const QUrl& serverUrl, QObject* parent )
+{
     QTcpSocket* socket = new QTcpSocket( parent );
-    socket->connectToHost( ficheHost, fichePort );
+    socket->connectToHost( serverUrl.host(), serverUrl.port() );
 
     if ( !socket->waitForConnected() )
     {
@@ -61,7 +48,7 @@ ficheLogUpload( QObject* parent )
         return QString();
     }
 
-    cDebug() << "Connected to paste server";
+    cDebug() << "Connected to paste server" << serverUrl.host();
 
     socket->write( pasteData );
 
@@ -72,7 +59,7 @@ ficheLogUpload( QObject* parent )
         return QString();
     }
 
-    cDebug() << "Paste data written to paste server";
+    cDebug() << Logger::SubEntry << "Paste data written to paste server";
 
     if ( !socket->waitForReadyRead() )
     {
@@ -81,35 +68,52 @@ ficheLogUpload( QObject* parent )
         return QString();
     }
 
-    cDebug() << "Reading response from paste server";
-
-    char resp[ 1024 ];
-    resp[ 0 ] = '\0';
-    qint64 nBytesRead = socket->readLine( resp, 1024 );
+    cDebug() << Logger::SubEntry << "Reading response from paste server";
+    QByteArray responseText = socket->readLine( 1024 );
     socket->close();
 
-    QUrl pasteUrl = QUrl( QString( resp ).trimmed(), QUrl::StrictMode );
-    QString pasteUrlStr = pasteUrl.toString();
-    QRegularExpression pasteUrlRegex( "^http[s]?://" + ficheHost );
-    QString pasteUrlMsg = QString( pasteUrlFmt ).arg( pasteUrlStr );
-
-    if ( nBytesRead >= 8 && pasteUrl.isValid() && pasteUrlRegex.match( pasteUrlStr ).hasMatch() )
+    QUrl pasteUrl = QUrl( QString( responseText ).trimmed(), QUrl::StrictMode );
+    if ( pasteUrl.isValid() && pasteUrl.host() == serverUrl.host() )
     {
-        QClipboard* clipboard = QApplication::clipboard();
-        clipboard->setText(pasteUrlStr, QClipboard::Clipboard);
-
-        if (clipboard->supportsSelection())
-        {
-             clipboard->setText(pasteUrlStr, QClipboard::Selection);
-        }
+        cDebug() << Logger::SubEntry << "Paste server results:" << pasteUrl;
+        return pasteUrl.toString();
     }
     else
     {
         cError() << "No data from paste server";
         return QString();
     }
-
-    cDebug() << "Paste server results:" << pasteUrlMsg;
-    return pasteUrlMsg;
 }
-}  // namespace CalamaresUtils
+
+QString
+CalamaresUtils::Paste::doLogUpload( QObject* parent )
+{
+    auto [ type, serverUrl ] = Calamares::Branding::instance()->uploadServer();
+    if ( !serverUrl.isValid() )
+    {
+        cWarning() << "Upload configure with invalid URL";
+        return QString();
+    }
+    if ( type == Calamares::Branding::UploadServerType::None )
+    {
+        // Early return to avoid reading the log file
+        return QString();
+    }
+
+    QByteArray pasteData = logFileContents();
+    if ( pasteData.isEmpty() )
+    {
+        // An error has already been logged
+        return QString();
+    }
+
+    switch ( type )
+    {
+    case Calamares::Branding::UploadServerType::None:
+        cWarning() << "No upload configured.";
+        return QString();
+    case Calamares::Branding::UploadServerType::Fiche:
+        return ficheLogUpload( pasteData, serverUrl, parent );
+    }
+    return QString();
+}

src/libcalamaresui/utils/Paste.h

@@ -10,21 +10,21 @@
 #ifndef UTILS_PASTE_H
 #define UTILS_PASTE_H
 
-#include<QStringList>
+#include <QString>
 
 class QObject;
-class QString;
 
 namespace CalamaresUtils
 {
+namespace Paste
+{
 
 /** @brief Send the current log file to a pastebin
  *
  * Returns the (string) URL that the pastebin gives us.
  */
-QString ficheLogUpload( QObject* parent );
-
-extern QStringList UploadServersList;
+QString doLogUpload( QObject* parent );
+}  // namespace Paste
 
 }  // namespace CalamaresUtils
 

src/libcalamaresui/utils/TestPaste.cpp

@@ -0,0 +1,67 @@
+/* === This file is part of Calamares - <https://calamares.io> ===
+ *
+ *   SPDX-FileCopyrightText: 2021 Adriaan de Groot <groot@kde.org>
+ *   SPDX-License-Identifier: GPL-3.0-or-later
+ *
+ *
+ *   Calamares is Free Software: see the License-Identifier above.
+ *
+ *
+ */
+
+#include "Paste.h"
+
+#include "utils/Logger.h"
+
+#include <QDateTime>
+#include <QtTest/QtTest>
+
+extern QByteArray logFileContents();
+extern QString ficheLogUpload( const QByteArray& pasteData, const QUrl& serverUrl, QObject* parent );
+
+class TestPaste : public QObject
+{
+    Q_OBJECT
+
+public:
+    TestPaste() {}
+    ~TestPaste() override {}
+
+private Q_SLOTS:
+    void testGetLogFile();
+    void testFichePaste();
+};
+
+void
+TestPaste::testGetLogFile()
+{
+    // This test assumes nothing **else** has set up logging yet
+    QByteArray b = logFileContents();
+    QVERIFY( b.isEmpty() );
+
+    Logger::setupLogLevel( Logger::LOGDEBUG );
+    Logger::setupLogfile();
+
+    b = logFileContents();
+    QVERIFY( !b.isEmpty() );
+}
+
+void
+TestPaste::testFichePaste()
+{
+    QString blabla( "the quick brown fox tested Calamares and found it rubbery" );
+    QDateTime now = QDateTime::currentDateTime();
+
+    QByteArray d = ( blabla + now.toString() ).toUtf8();
+    QString s = ficheLogUpload( d, QUrl( "http://termbin.com:9999" ), nullptr );
+
+    cDebug() << "Paste data to" << s;
+    QVERIFY( !s.isEmpty() );
+}
+
+
+QTEST_GUILESS_MAIN( TestPaste )
+
+#include "utils/moc-warnings.h"
+
+#include "TestPaste.moc"