[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-commits
Subject:    [amarok] src: Fix: prevent potential issues with QString::arg
From:       Ralf Engels <ralf-engels () gmx ! de>
Date:       2015-01-31 8:31:27
Message-ID: E1YHTSh-0007Lz-AW () scm ! kde ! org
[Download RAW message or body]

Git commit d9b1f28598cadea172d1940ec4b6755fd13ca3e0 by Ralf Engels.
Committed on 30/01/2015 at 13:58.
Pushed by rengels into branch 'master'.

Fix: prevent potential issues with QString::arg

When using several "arg" one must make sure that there is not
a string inserted with %1 or else the next .arg calls would
also replace it.

Used one arg or just stream operations when usefull.

Was not even aware of this dangerous behavour of QString before
and it was also not mentioned in the documentation.

M  +4    -2    src/amarokurls/AmarokUrl.cpp
M  +2    -2    src/amarokurls/BookmarkGroup.cpp
M  +2    -2    src/browsers/playlistbrowser/PlaylistBrowserModel.cpp
M  +1    -1    src/context/applets/upcomingevents/UpcomingEventsWidget.cpp
M  +2    -2    src/context/applets/wikipedia/WikipediaApplet.cpp
M  +1    -1    src/core-impl/playlists/types/file/PlaylistFileSupport.cpp
M  +3    -3    src/covermanager/CoverFetcher.cpp
M  +3    -2    src/moodbar/MoodbarManager.cpp
M  +8    -8    src/playlistmanager/SyncedPlaylist.cpp
M  +3    -4    src/playlistmanager/sql/SqlPlaylistGroup.cpp
M  +1    -1    src/scripting/scriptconsole/ScriptConsole.cpp
M  +3    -3    src/scripting/scriptconsole/ScriptConsoleItem.cpp
M  +2    -2    src/scripting/scriptengine/ScriptingDefines.cpp
M  +1    -1    src/services/lastfm/AvatarDownloader.cpp
M  +1    -1    src/services/lastfm/LastFmService.cpp

http://commits.kde.org/amarok/d9b1f28598cadea172d1940ec4b6755fd13ca3e0

diff --git a/src/amarokurls/AmarokUrl.cpp b/src/amarokurls/AmarokUrl.cpp
index 2981512..e64a1ed 100644
--- a/src/amarokurls/AmarokUrl.cpp
+++ b/src/amarokurls/AmarokUrl.cpp
@@ -151,7 +151,8 @@ bool AmarokUrl::saveToDb()
         //update existing
         debug() << "Updating bookmark";
         QString query = "UPDATE bookmarks SET parent_id=%1, name='%2', url='%3', \
                description='%4', custom='%5' WHERE id=%6;";
-        query = query.arg( QString::number( parentId ) ).arg( sql->escape( m_name ), \
sql->escape( url() ), sql->escape( m_description ), sql->escape( m_customValue ) , \
QString::number( m_id ) ); +        query = query.arg( QString::number( parentId ), \
sql->escape( m_name ), sql->escape( url() ), +                           sql->escape( \
m_description ), sql->escape( m_customValue ), QString::number( m_id ) );  \
StorageManager::instance()->sqlStorage()->query( query );  }
     else
@@ -159,7 +160,8 @@ bool AmarokUrl::saveToDb()
         //insert new
         debug() << "Creating new bookmark in the db";
         QString query = "INSERT INTO bookmarks ( parent_id, name, url, description, \
                custom ) VALUES ( %1, '%2', '%3', '%4', '%5' );";
-        query = query.arg( QString::number( parentId ), sql->escape( m_name ), \
sql->escape( url() ), sql->escape( m_description ), sql->escape( m_customValue ) ); + \
query = query.arg( QString::number( parentId ), sql->escape( m_name ), sql->escape( \
url() ), +                           sql->escape( m_description ), sql->escape( \
                m_customValue ) );
         m_id = StorageManager::instance()->sqlStorage()->insert( query, NULL );
     }
 
diff --git a/src/amarokurls/BookmarkGroup.cpp b/src/amarokurls/BookmarkGroup.cpp
index a054302..ca1f607 100644
--- a/src/amarokurls/BookmarkGroup.cpp
+++ b/src/amarokurls/BookmarkGroup.cpp
@@ -100,14 +100,14 @@ void BookmarkGroup::save()
     if ( m_dbId != -1 ) {
         //update existing
         QString query = "UPDATE bookmark_groups SET parent_id=%1, name='%2', \
                description='%3', custom='%4%' WHERE id=%5;";
-        query = query.arg( QString::number( parentId ) ).arg( m_name ).arg( \
m_description ).arg( m_customType ).arg( QString::number( m_dbId ) ); +        query \
= query.arg( QString::number( parentId ), m_name, m_description, m_customType, \
QString::number( m_dbId ) );  StorageManager::instance()->sqlStorage()->query( query \
);  }
     else
     {
         //insert new
         QString query = "INSERT INTO bookmark_groups ( parent_id, name, description, \
                custom) VALUES ( %1, '%2', '%3', '%4' );";
-        query = query.arg( QString::number( parentId ) ).arg( m_name ).arg( \
m_description ).arg( m_customType ); +        query = query.arg( QString::number( \
                parentId ), m_name, m_description, m_customType );
         m_dbId = StorageManager::instance()->sqlStorage()->insert( query, NULL );
 
     }
diff --git a/src/browsers/playlistbrowser/PlaylistBrowserModel.cpp \
b/src/browsers/playlistbrowser/PlaylistBrowserModel.cpp index 32d3d8b..f43cc72 100644
--- a/src/browsers/playlistbrowser/PlaylistBrowserModel.cpp
+++ b/src/browsers/playlistbrowser/PlaylistBrowserModel.cpp
@@ -205,7 +205,7 @@ PlaylistBrowserModel::setData( const QModelIndex &idx, const \
QVariant &value, in  if( !track )
                         return false;
                     debug() << QString( "Copy track \"%1\" to \"%2\"." )
-                            .arg( track->prettyName() ).arg( provider->prettyName() \
); +                            .arg( track->prettyName(),  provider->prettyName() );
     //                return !provider->addTrack( track ).isNull();
                     provider->addTrack( track ); //ignore result since \
UmsPodcastProvider returns NULL  return true;
@@ -223,7 +223,7 @@ PlaylistBrowserModel::setData( const QModelIndex &idx, const \
QVariant &value, in  }
 
                     debug() << QString( "Copy playlist \"%1\" to \"%2\"." )
-                            .arg( playlist->prettyName() ).arg( \
provider->prettyName() ); +                            .arg( playlist->prettyName(), \
provider->prettyName() );  
                     return !provider->addPlaylist( playlist ).isNull();
                 }
diff --git a/src/context/applets/upcomingevents/UpcomingEventsWidget.cpp \
b/src/context/applets/upcomingevents/UpcomingEventsWidget.cpp index 323ec1e..3ecd752 \
                100644
--- a/src/context/applets/upcomingevents/UpcomingEventsWidget.cpp
+++ b/src/context/applets/upcomingevents/UpcomingEventsWidget.cpp
@@ -261,7 +261,7 @@ UpcomingEventsWidget::setDate( const KDateTime &date )
 void
 UpcomingEventsWidget::setLocation( const LastFmLocationPtr &loc )
 {
-    QString text = QString( "%1, %2" ).arg( loc->city ).arg( loc->country );
+    QString text = QString( "%1, %2" ).arg( loc->city, loc->country );
     if( !loc->street.isEmpty() )
         text.prepend( loc->street + ", " );
     QLabel *locLabel = static_cast<QLabel*>( m_location->widget() );
diff --git a/src/context/applets/wikipedia/WikipediaApplet.cpp \
b/src/context/applets/wikipedia/WikipediaApplet.cpp index 21365f2..2ceb2b0 100644
--- a/src/context/applets/wikipedia/WikipediaApplet.cpp
+++ b/src/context/applets/wikipedia/WikipediaApplet.cpp
@@ -63,7 +63,7 @@ WikipediaAppletPrivate::parseWikiLangXml( const QByteArray &data )
             {
                 const QString &prefix = a.value("prefix").toString();
                 const QString &language = a.value("language").toString();
-                const QString &display = QString( "[%1] %2" ).arg( prefix ).arg( \
language ); +                const QString &display = QString( "[%1] %2" ).arg( \
                prefix, language );
                 QListWidgetItem *item = new QListWidgetItem( display, 0 );
                 // The urlPrefix is the lang code infront of the wikipedia host
                 // url. It is mostly the same as the "prefix" attribute but in
@@ -245,7 +245,7 @@ WikipediaAppletPrivate::_loadSettings()
         QListWidgetItem *item = listWidget->item( i );
         const QString &prefix = item->data( PrefixRole ).toString();
         const QString &urlPrefix = item->data( UrlPrefixRole ).toString();
-        QString concat = QString("%1:%2").arg( prefix ).arg( urlPrefix );
+        QString concat = QString("%1:%2").arg( prefix, urlPrefix );
         list << (prefix == urlPrefix ? prefix : concat);
     }
     langList = list;
diff --git a/src/core-impl/playlists/types/file/PlaylistFileSupport.cpp \
b/src/core-impl/playlists/types/file/PlaylistFileSupport.cpp index 522f4da..3fbde45 \
                100644
--- a/src/core-impl/playlists/types/file/PlaylistFileSupport.cpp
+++ b/src/core-impl/playlists/types/file/PlaylistFileSupport.cpp
@@ -154,5 +154,5 @@ Playlists::newPlaylistFilePath( const QString &fileExtension )
     while( QFileInfo( url.path() ).exists() )
         url.setFileName( fileName.subs( ++trailingNumber ).toString() );
 
-    return KUrl( QString( "%1.%2" ).arg( url.path() ).arg( fileExtension ) );
+    return KUrl( QString( "%1.%2" ).arg( url.path(), fileExtension ) );
 }
diff --git a/src/covermanager/CoverFetcher.cpp b/src/covermanager/CoverFetcher.cpp
index 5f85807..704be17 100644
--- a/src/covermanager/CoverFetcher.cpp
+++ b/src/covermanager/CoverFetcher.cpp
@@ -80,8 +80,8 @@ void
 CoverFetcher::manualFetch( Meta::AlbumPtr album )
 {
     debug() << QString("Adding interactive cover fetch for: '%1' from %2")
-        .arg( album->name() )
-        .arg( Amarok::config("Cover Fetcher").readEntry("Interactive Image Source", \
"LastFm") ); +        .arg( album->name(),
+              Amarok::config("Cover Fetcher").readEntry("Interactive Image Source", \
"LastFm") );  switch( fetchSource() )
     {
     case CoverFetch::LastFm:
@@ -124,7 +124,7 @@ void
 CoverFetcher::queueQuery( Meta::AlbumPtr album, const QString &query, int page )
 {
     m_queue->addQuery( query, fetchSource(), page, album );
-    debug() << QString( "Queueing cover fetch query: '%1' (page %2)" ).arg( query \
).arg( page ); +    debug() << QString( "Queueing cover fetch query: '%1' (page %2)" \
).arg( query, QString::number( page ) );  }
 
 void
diff --git a/src/moodbar/MoodbarManager.cpp b/src/moodbar/MoodbarManager.cpp
index 2730de3..9480cbd 100644
--- a/src/moodbar/MoodbarManager.cpp
+++ b/src/moodbar/MoodbarManager.cpp
@@ -152,9 +152,10 @@ QPixmap MoodbarManager::getMoodbar( Meta::TrackPtr track, int \
width, int height,  
 
     //Do we already have this pixmap cached?
-    const QString pixmapKey = QString( "mood:%1-%2x%3%4" ).arg( track->uidUrl() \
).arg(width).arg(height).arg( rtl?"r":"" ); +    const QString pixmapKey = QString( \
"mood:%1-%2x%3%4" ).arg( track->uidUrl(), QString::number( width ), +                 \
QString::number( height ), QString( rtl?"r":"" ) );  QPixmap moodbar;
-    
+
     if( m_cache->findPixmap( pixmapKey, &moodbar ) )
         return moodbar;
         
diff --git a/src/playlistmanager/SyncedPlaylist.cpp \
b/src/playlistmanager/SyncedPlaylist.cpp index 8bf21ba..677539e 100644
--- a/src/playlistmanager/SyncedPlaylist.cpp
+++ b/src/playlistmanager/SyncedPlaylist.cpp
@@ -33,7 +33,7 @@ SyncedPlaylist::SyncedPlaylist( Playlists::PlaylistPtr playlist )
 KUrl
 SyncedPlaylist::uidUrl() const
 {
-    return KUrl( QString( "amarok-syncedplaylist://%1" ).arg( \
m_playlists.first()->name() ) ); +    return KUrl( QString( \
"amarok-syncedplaylist://" ) +  m_playlists.first()->name() );  }
 
 QString
@@ -195,8 +195,8 @@ SyncedPlaylist::syncNeeded() const
     Playlists::PlaylistPtr master = *i;
     int masterTrackCount = master->trackCount();
     ++i; //From now on its only slaves on the iterator
-    debug() << QString( "Master Playlist: %1 - %2" ).arg( master->name() ).arg( \
                master->uidUrl().url() );
-    debug() << QString( "Master track count: %1" ).arg( masterTrackCount );
+    debug() << "Master Playlist: " << master->name() << " - " << \
master->uidUrl().url(); +    debug() << "Master track count: " << masterTrackCount;
 
     for( ;i != m_playlists.end(); ++i)
     {
@@ -204,11 +204,11 @@ SyncedPlaylist::syncNeeded() const
         //Playlists::PlaylistPtr slave = i.next();
         Playlists::PlaylistPtr slave = *i;
 
-        debug() << QString( "Slave Playlist: %1 - %2" ).arg( slave->name() ).arg( \
slave->uidUrl().url() ); +        debug() << "Slave Playlist: " << slave->name() << " \
- " << slave->uidUrl().url();  if( masterTrackCount != -1 )
         {
             int slaveTrackCount = slave->trackCount();
-            debug() << QString( "Slave track count: %1" ).arg( slaveTrackCount );
+            debug() << "Slave track count: " << slaveTrackCount;
             //If the number of tracks is different a sync is certainly required
             if( slaveTrackCount != -1 && slaveTrackCount != masterTrackCount )
                 return true;
@@ -243,7 +243,7 @@ SyncedPlaylist::doSync() const
     QListIterator<TrackPtr> m( master->tracks() );
     //debug: print list
     int position = 0;
-    debug() << QString( "Master Playlist: %1 - %2" ).arg( master->name() ).arg( \
master->uidUrl().url() ); +    debug() << "Master Playlist: " << master->name() << " \
- " << master->uidUrl().url();  while( m.hasNext() )
         debug() << QString( "%1 : %2" ).arg( position++ ).arg( m.next()->name() );
     m.toFront();
@@ -254,7 +254,7 @@ SyncedPlaylist::doSync() const
         TrackList slaveTracks = slave->tracks();
         //debug: print list
         position = 0;
-        debug() << QString( "Slave Playlist: %1 - %2" ).arg( slave->name() ).arg( \
slave->uidUrl().url() ); +        debug() << "Slave Playlist: " << slave->name() << " \
- " << slave->uidUrl().url();  foreach( const TrackPtr track, slaveTracks )
             debug() << QString( "%1 : %2" ).arg( position++ ).arg( track->name() );
 
@@ -266,7 +266,7 @@ SyncedPlaylist::doSync() const
             if( position >= slaveTracks.size()
                     || track->uidUrl() != slaveTracks.at( position )->uidUrl() )
             {
-                debug() << QString( "insert %1 at %2" ).arg( track->name() ).arg( \
position ); +                debug() << QString( "insert %2 at %1" ).arg( position \
).arg( track->name() );  slave->addTrack( track, position );
 
                 slave->syncTrackStatus( position, track );
diff --git a/src/playlistmanager/sql/SqlPlaylistGroup.cpp \
b/src/playlistmanager/sql/SqlPlaylistGroup.cpp index 47c6baf..9267ebb 100644
--- a/src/playlistmanager/sql/SqlPlaylistGroup.cpp
+++ b/src/playlistmanager/sql/SqlPlaylistGroup.cpp
@@ -71,8 +71,8 @@ SqlPlaylistGroup::save()
         //update existing
         QString query = "UPDATE playlist_groups SET parent_id=%1, name='%2', \
                 description='%3' WHERE id=%4;";
-        query = query.arg( QString::number( parentId ) ).arg( m_name ).arg(
-            m_description ).arg( QString::number( m_dbId ) );
+        query = query.arg( QString::number( parentId ), m_name,
+                           m_description, QString::number( m_dbId ) );
         sqlStorage->query( query );
     }
     else
@@ -80,8 +80,7 @@ SqlPlaylistGroup::save()
         //insert new
         QString query = "INSERT INTO playlist_groups ( parent_id, name, \
                 description) VALUES ( %1, '%2', '%3' );";
-        query = query.arg( QString::number( parentId ) ).arg( m_name ).arg(
-            m_description );
+        query = query.arg( QString::number( parentId ), m_name, m_description );
         m_dbId = sqlStorage->insert( query, NULL );
     }
 }
diff --git a/src/scripting/scriptconsole/ScriptConsole.cpp \
b/src/scripting/scriptconsole/ScriptConsole.cpp index 15721fa..6edd9a2 100644
--- a/src/scripting/scriptconsole/ScriptConsole.cpp
+++ b/src/scripting/scriptconsole/ScriptConsole.cpp
@@ -231,7 +231,7 @@ ScriptConsole::createScriptItem( const QString &script )
     do
     {
         scriptName = QString( "Script-%1" ).arg( qrand() );
-        scriptPath =  QString( "%1/%2" ).arg( m_savePath ).arg( scriptName );
+        scriptPath =  QString( "%1/%2" ).arg( m_savePath, scriptName );
     } while ( QDir( scriptPath ).exists() );
     QDir().mkdir( scriptPath );
 
diff --git a/src/scripting/scriptconsole/ScriptConsoleItem.cpp \
b/src/scripting/scriptconsole/ScriptConsoleItem.cpp index 16cc18e..61bc0c4 100644
--- a/src/scripting/scriptconsole/ScriptConsoleItem.cpp
+++ b/src/scripting/scriptconsole/ScriptConsoleItem.cpp
@@ -77,7 +77,7 @@ ScriptConsoleItem::createSpecFile( const QString &name, const \
QString &category,  "\nX-KDE-PluginInfo-Version=1.0"
                             "\nX-KDE-PluginInfo-Category=%2"
                             "\nX-KDE-PluginInfo-Depends=Amarok2.0"
-                            "\nX-KDE-PluginInfo-EnabledByDefault=false\n" ).arg( \
name ).arg( category ); +                            \
"\nX-KDE-PluginInfo-EnabledByDefault=false\n" ).arg( name, category );  
     QString specPath = QString( "%1/script.spec" ).arg( path );
     QFile file( specPath );
@@ -140,8 +140,8 @@ QString
 ScriptConsoleItem::handleError( QScriptEngine *engine )
 {
     QString errorString = QString( "Script Error: %1 (line: %2)" )
-                        .arg( engine->uncaughtException().toString() )
-                        .arg( engine->uncaughtExceptionLineNumber() );
+                        .arg( engine->uncaughtException().toString(),
+                              QString::number( engine->uncaughtExceptionLineNumber() \
) );  return errorString;
 }
 
diff --git a/src/scripting/scriptengine/ScriptingDefines.cpp \
b/src/scripting/scriptengine/ScriptingDefines.cpp index f7e645d..97c036b 100644
--- a/src/scripting/scriptengine/ScriptingDefines.cpp
+++ b/src/scripting/scriptengine/ScriptingDefines.cpp
@@ -39,13 +39,13 @@ AmarokScriptEngine::AmarokScriptEngine( QObject *parent )
 void
 AmarokScriptEngine::setDeprecatedProperty( const QString &parent, const QString \
&name, const QScriptValue &property )  {
-    const QString objName = QString( "%1%2" ).arg( name ).arg( qrand() );
+    const QString objName = QString( "%1%2" ).arg( name, QString::number( qrand() ) \
                );
     globalObject().property( internalObject ).setProperty( objName, property, \
                QScriptValue::ReadOnly | QScriptValue::SkipInEnumeration );
     const QString command = QString( "Object.defineProperty( %1, \"%2\", {get : \
                function(){ var iobj= %3; iobj.invokableDeprecatedCall(\""
                                                                             " %1.%2 \
                \"); return iobj.%4; },\
                                                                             \
                enumerable : true,\
                                                                             \
                configurable : false} );" )
-                                    .arg( parent ).arg( name ).arg( internalObject \
).arg( objName ); +                                    .arg( parent, name, \
internalObject, objName );  evaluate( command );
 }
 
diff --git a/src/services/lastfm/AvatarDownloader.cpp \
b/src/services/lastfm/AvatarDownloader.cpp index b54b252..182b234 100644
--- a/src/services/lastfm/AvatarDownloader.cpp
+++ b/src/services/lastfm/AvatarDownloader.cpp
@@ -55,7 +55,7 @@ AvatarDownloader::downloaded( const KUrl &url, QByteArray data, \
NetworkAccessMan  emit avatarDownloaded( username, avatar );
     }
     else
-        debug() << QString("Error: failed to download %1'savatar: \
%1").arg(username).arg(e.description); +        debug() << QString( "Error: failed to \
download %1'savatar: %2" ).arg( username, e.description );  }
 
 #include "AvatarDownloader.moc"
diff --git a/src/services/lastfm/LastFmService.cpp \
b/src/services/lastfm/LastFmService.cpp index 48a9258..93ab993 100644
--- a/src/services/lastfm/LastFmService.cpp
+++ b/src/services/lastfm/LastFmService.cpp
@@ -211,7 +211,7 @@ LastFmService::slotReconfigure()
             m_authenticateReply = 0;
         }
 
-        const QString authToken = md5( QString( "%1%2" ).arg( m_config->username() \
).arg( +        const QString authToken = md5( QString( "%1%2" ).arg( \
m_config->username(),  md5( m_config->password().toUtf8() ) ).toUtf8() );
         QMap<QString, QString> query;
         query[ "method" ] = "auth.getMobileSession";


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic