Bug 116371

I can confirm this in KDE 3.5 (Kubuntu packages). If any error appears when deleting a folder from trash (like "no access rights"), the folder is removed from ~/.local/Trash/info and thus not visible in trash:/ anymore, but the folder is still in ~/.local/Trash/files, and thus filling up hardware space, which a newbie would never be able to clean up.

I confirm this bug too in Kubuntu (KDE 3.5.1). The Trash (in KDE) is empty, but in ~/.local/share/Trash/files, there are a lot of files!

> I confirm this bug too in Kubuntu (KDE 3.5.1). The Trash (in KDE) is empty, but in ~/.local/share/Trash/files, there are a lot of files!


And in ~/.local/share/Trash/info/ ?

~/.local/share/Trash/info/ is empty.

> ~/.local/share/Trash/info/ is empty.

So it's normal that KDE sees the trash as empty.
Now the question is: how can a file remain in files/ and while its info got deleted...
This isn't supposed to happen, of course.
Are those files in files/ owned by the same user, i.e. deletable?
Can you clean up files/ and use the kde trash system again until you find a way 
to end up with a file in files/ but not in info/ ?

(Sorry for my bad English, I'm a young French)
I erased /.local/share/Trash. When I removed files in Trash, and used the KDE trash... there is nothing in ~/.local/share/Trash/info/ and ~/.local/share/Trash/files/!
Before removing /.local/share/Trash, in the directory files, there was lot of files, and nothing in info.

This bug looks fixed afted an update of kubuntu !

Kubuntu 6.06 KDE 3.5.3

there is still a bug with the trash: when emptying the trash, the icon says "empty", but a lot of files & folders stay in ~/.local/share/trash/.

Tonight, i had to clean my HD running out of space, I found about 2.2Go in the ~/.local/share/trash/ folder. But the trash seemed to be empty according to the systray icon and .Trash folder.

I'm using Fat32 mounted USB-Hard-drive to share files with win32 cuptuters.

bug was submitted to (k)ubuntu launchpad with a screen capture:
https://launchpad.net/distros/ubuntu/+source/meta-kde/+bug/30842

best regards

Emmanuel

*** Bug 130780 has been marked as a duplicate of this bug. ***

*** Bug 126357 has been marked as a duplicate of this bug. ***

SVN commit 583812 by dfaure:

Do the equivalent of a "chmod -R a+w" on the directory before attempting to delete it from the trash
 (e.g. when emptying the trash or deleting a single dir from the trash).
This helps fixes readonly dirs not being cleaned up.

Hmm, the problem with root-owned dirs isn't solved though. In fact there is *no* way for a user
to delete such a dir (if not empty)... What then? I guess the info file should stay so that the user
can see that the trash isn't empty, but he/she will have to ask root to clean up the directory, there's
no other way. This is all happening because of the atomic move on the same partition, which can
bring along files/dirs owned by root or other users along with it.

CCBUG: 116371


 M  +48 -7     testtrash.cpp  
 M  +2 -0      testtrash.h  
 M  +25 -8     trashimpl.cpp  
 M  +1 -1      trashimpl.h  


--- branches/KDE/3.5/kdebase/kioslave/trash/testtrash.cpp #583811:583812
@@ -98,6 +98,11 @@
     return QDir::homeDirPath() + "/.kde/testtrash/";
 }
 
+QString TestTrash::readOnlyDirPath() const
+{
+    return homeTmpDir() + QString( "readonly" );
+}
+
 QString TestTrash::otherTmpDir() const
 {
     // This one needs to be on another partition
@@ -191,8 +196,16 @@
         kdWarning() << "No writable partition other than $HOME found, some tests will be skipped" << endl;
 
     // Start with a clean base dir
-    KIO::NetAccess::del( homeTmpDir(), 0 );
-    KIO::NetAccess::del( otherTmpDir(), 0 );
+    if ( QFileInfo( homeTmpDir() ).exists() ) {
+        bool ok = KIO::NetAccess::del( homeTmpDir(), 0 );
+        if ( !ok )
+            kdFatal() << "Couldn't delete " << homeTmpDir() << endl;
+    }
+    if ( QFileInfo( otherTmpDir() ).exists() ) {
+        bool ok = KIO::NetAccess::del( otherTmpDir(), 0 );
+        if ( !ok )
+            kdFatal() << "Couldn't delete " << otherTmpDir() << endl;
+    }
     QDir dir; // TT: why not a static method?
     bool ok = dir.mkdir( homeTmpDir() );
     if ( !ok )
@@ -206,6 +219,7 @@
 
 void TestTrash::cleanTrash()
 {
+    kdDebug() << k_funcinfo << endl;
     // Start with a relatively clean trash too
     removeFile( m_trashDir, "/info/fileFromHome.trashinfo" );
     removeFile( m_trashDir, "/files/fileFromHome" );
@@ -227,6 +241,7 @@
     removeFile( m_trashDir, "/files/brokenSymlinkFromHome" );
     removeFile( m_trashDir, "/info/trashDirFromHome.trashinfo" );
     removeFile( m_trashDir, "/files/trashDirFromHome/testfile" );
+    removeFile( m_trashDir, "/info/readonly.trashinfo" );
     removeDir( m_trashDir, "/files/trashDirFromHome" );
     removeFile( m_trashDir, "/info/trashDirFromHome_1.trashinfo" );
     removeFile( m_trashDir, "/files/trashDirFromHome_1/testfile" );
@@ -234,6 +249,7 @@
     removeFile( m_trashDir, "/info/trashDirFromOther.trashinfo" );
     removeFile( m_trashDir, "/files/trashDirFromOther/testfile" );
     removeDir( m_trashDir, "/files/trashDirFromOther" );
+    KIO::NetAccess::del( m_trashDir + "/files/readonly", 0 );
     // for trashDirectoryOwnedByRoot
     KIO::NetAccess::del( m_trashDir + "/files/cups", 0 );
     KIO::NetAccess::del( m_trashDir + "/files/boot", 0 );
@@ -254,6 +270,7 @@
     trashUtf8FileFromHome();
 #endif
     trashUmlautFileFromHome();
+    trashReadOnlyDirFromHome();
     testTrashNotEmpty();
     trashFileFromOther();
     trashFileIntoOtherPartition();
@@ -599,14 +616,16 @@
 {
     kdDebug() << k_funcinfo << fileId << endl;
     // setup
-    QDir dir;
-    bool ok = dir.mkdir( origPath );
-    Q_ASSERT( ok );
+    if ( !QFileInfo( origPath ).exists() ) {
+        QDir dir;
+        bool ok = dir.mkdir( origPath );
+        Q_ASSERT( ok );
+    }
     createTestFile( origPath + "/testfile" );
     KURL u; u.setPath( origPath );
 
     // test
-    ok = KIO::NetAccess::move( u, "trash:/" );
+    bool ok = KIO::NetAccess::move( u, "trash:/" );
     assert( ok );
     if ( origPath.startsWith( "/tmp" ) && m_tmpIsWritablePartition ) {
         kdDebug() << " TESTS SKIPPED" << endl;
@@ -632,6 +651,23 @@
     trashDirectory( homeTmpDir() + dirName, dirName + "_1" );
 }
 
+void TestTrash::trashReadOnlyDirFromHome()
+{
+    kdDebug() << k_funcinfo << endl;
+    const QString dirName = readOnlyDirPath();
+    QDir dir;
+    bool ok = dir.mkdir( dirName );
+    Q_ASSERT( ok );
+    // #130780
+    const QString subDirPath = dirName + "/readonly_subdir";
+    ok = dir.mkdir( subDirPath );
+    Q_ASSERT( ok );
+    createTestFile( subDirPath + "/testfile_in_subdir" );
+    ::chmod( QFile::encodeName( subDirPath ), 0500 );
+
+    trashDirectory( dirName, "readonly" );
+}
+
 void TestTrash::trashDirectoryFromOther()
 {
     kdDebug() << k_funcinfo << endl;
@@ -1136,7 +1172,7 @@
     // ## Even though we use a custom XDG_DATA_HOME value, emptying the
     // trash would still empty the other trash directories in other partitions.
     // So we can't activate this test by default.
-#if 0
+#if 1
     kdDebug() << k_funcinfo << endl;
     QByteArray packedArgs;
     QDataStream stream( packedArgs, IO_WriteOnly );
@@ -1149,6 +1185,11 @@
     assert( cfg.hasGroup( "Status" ) );
     cfg.setGroup( "Status" );
     assert( cfg.readBoolEntry( "Empty", false ) == true );
+
+    assert( !QFile::exists( m_trashDir + "/files/fileFromHome" ) );
+    assert( !QFile::exists( m_trashDir + "/files/readonly" ) );
+    assert( !QFile::exists( m_trashDir + "/info/readonly.trashinfo" ) );
+
 #else
     kdDebug() << k_funcinfo << " : SKIPPED" << endl;
 #endif
--- branches/KDE/3.5/kdebase/kioslave/trash/testtrash.h #583811:583812
@@ -50,6 +50,7 @@
     void trashSymlinkFromOther();
     void trashBrokenSymlinkFromHome();
     void trashDirectoryFromHome();
+    void trashReadOnlyDirFromHome();
     void trashDirectoryFromOther();
     void trashDirectoryOwnedByRoot();
 
@@ -100,6 +101,7 @@
     QString otherTmpDir() const;
     QString utf8FileName() const;
     QString umlautFileName() const;
+    QString readOnlyDirPath() const;
 
     QString m_trashDir;
 
--- branches/KDE/3.5/kdebase/kioslave/trash/trashimpl.cpp #583811:583812
@@ -30,6 +30,8 @@
 #include <kstandarddirs.h>
 #include <kglobalsettings.h>
 #include <kmountpoint.h>
+#include <kfileitem.h>
+#include <kio/chmodjob.h>
 
 #include <dcopref.h>
 
@@ -181,7 +183,7 @@
     if ( allOK ) {
         // We need to remove the old one, otherwise the desktop will have two trashcans...
         kdDebug() << "Trash migration: all OK, removing old trash directory" << endl;
-        synchronousDel( oldTrashDir, false );
+        synchronousDel( oldTrashDir, false, true );
     }
 }
 
@@ -321,7 +323,7 @@
         if ( QFileInfo( dest ).isFile() )
             QFile::remove( dest );
         else
-            synchronousDel( dest, false );
+            synchronousDel( dest, false, true );
         return false;
     }
     fileAdded();
@@ -477,18 +479,33 @@
 
     QFile::remove( info );
 
-    if ( !synchronousDel( file, true ) )
+    if ( !synchronousDel( file, true, QFileInfo(file).isDir() ) )
         return false;
     fileRemoved();
     return true;
 }
 
-bool TrashImpl::synchronousDel( const QString& path, bool setLastErrorCode )
+bool TrashImpl::synchronousDel( const QString& path, bool setLastErrorCode, bool isDir )
 {
     const int oldErrorCode = m_lastErrorCode;
     const QString oldErrorMsg = m_lastErrorMessage;
     KURL url;
     url.setPath( path );
+
+    // First ensure that all dirs have u+w permissions,
+    // otherwise we won't be able to delete files in them (#130780).
+    if ( isDir ) {
+        kdDebug() << k_funcinfo << "chmod'ing " << url << endl;
+        KFileItem fileItem( url, "inode/directory", KFileItem::Unknown );
+        KFileItemList fileItemList;
+        fileItemList.append( &fileItem );
+        KIO::ChmodJob* chmodJob = KIO::chmod( fileItemList, 0777, 0222, QString::null, QString::null, true /*recursive*/, false /*showProgressInfo*/ );
+        connect( chmodJob, SIGNAL( result(KIO::Job *) ),
+                 this, SLOT( jobFinished(KIO::Job *) ) );
+        qApp->eventLoop()->enterLoop();
+    }
+
+    kdDebug() << k_funcinfo << "deleting " << url << endl;
     KIO::DeleteJob *job = KIO::del( url, false, false );
     connect( job, SIGNAL( result(KIO::Job *) ),
              this, SLOT( jobFinished(KIO::Job *) ) );
@@ -511,11 +528,11 @@
     for ( ; it != m_trashDirectories.end() ; ++it ) {
         QDir dir;
         const QString infoPath = it.data() + "/info";
-        // TODO show errors (with warning()), e.g. when permission denied?
-        synchronousDel( infoPath, false );
+        synchronousDel( infoPath, false, true );
         dir.mkdir( infoPath );
         const QString filesPath = it.data() + "/files";
-        synchronousDel( filesPath, false );
+        // TODO show errors (with warning()), e.g. when permission denied?
+        synchronousDel( filesPath, false, true );
         dir.mkdir( filesPath );
     }
     fileRemoved();
@@ -763,7 +780,7 @@
     uid_t uid = getuid();
     KDE_struct_stat buff;
     // Minimum permissions required: write+execute for 'others', and sticky bit
-    int requiredBits = S_IWOTH | S_IXOTH | S_ISVTX;
+    uint requiredBits = S_IWOTH | S_IXOTH | S_ISVTX;
     if ( KDE_lstat( QFile::encodeName( rootTrashDir ), &buff ) == 0 ) {
         if ( (buff.st_uid == 0) // must be owned by root
              && (S_ISDIR(buff.st_mode)) // must be a dir
--- branches/KDE/3.5/kdebase/kioslave/trash/trashimpl.h #583811:583812
@@ -137,7 +137,7 @@
     QString trashDirectoryPath( int trashId ) const;
     QString topDirectoryPath( int trashId ) const;
 
-    bool synchronousDel( const QString& path, bool setLastErrorCode );
+    bool synchronousDel( const QString& path, bool setLastErrorCode, bool isDir );
 
     void scanTrashDirectories() const;
 

SVN commit 583856 by dfaure:

Only remove the entry (.trashinfo file) if the deletion succeeded; otherwise report the error.
This ensures that we don't end up with "trash appears to be empty but it still takes much hdd space".
BUG: 116371


 M  +4 -2      kio_trash.cpp  
 M  +1 -1      testtrash.cpp  
 M  +20 -16    trashimpl.cpp  
 M  +1 -1      trashimpl.h  


--- branches/KDE/3.5/kdebase/kioslave/trash/kio_trash.cpp #583855:583856
@@ -466,8 +466,10 @@
 
     switch (cmd) {
     case 1:
-        impl.emptyTrash();
-        finished();
+        if ( impl.emptyTrash() )
+            finished();
+        else
+            error( impl.lastErrorCode(), impl.lastErrorMessage() );
         break;
     case 2:
         impl.migrateOldTrash();
--- branches/KDE/3.5/kdebase/kioslave/trash/testtrash.cpp #583855:583856
@@ -1172,7 +1172,7 @@
     // ## Even though we use a custom XDG_DATA_HOME value, emptying the
     // trash would still empty the other trash directories in other partitions.
     // So we can't activate this test by default.
-#if 0
+#if 1
     kdDebug() << k_funcinfo << endl;
     QByteArray packedArgs;
     QDataStream stream( packedArgs, IO_WriteOnly );
--- branches/KDE/3.5/kdebase/kioslave/trash/trashimpl.cpp #583855:583856
@@ -477,10 +477,10 @@
         return false;
     }
 
-    QFile::remove( info );
-
     if ( !synchronousDel( file, true, QFileInfo(file).isDir() ) )
         return false;
+
+    QFile::remove( info );
     fileRemoved();
     return true;
 }
@@ -518,24 +518,28 @@
     return ok;
 }
 
-void TrashImpl::emptyTrash()
+bool TrashImpl::emptyTrash()
 {
     kdDebug() << k_funcinfo << endl;
-    if ( !m_trashDirectoriesScanned )
-        scanTrashDirectories();
-    // For each known trash directory...
-    TrashDirMap::const_iterator it = m_trashDirectories.begin();
-    for ( ; it != m_trashDirectories.end() ; ++it ) {
-        QDir dir;
-        const QString infoPath = it.data() + "/info";
-        synchronousDel( infoPath, false, true );
-        dir.mkdir( infoPath );
-        const QString filesPath = it.data() + "/files";
-        // TODO show errors (with warning()), e.g. when permission denied?
-        synchronousDel( filesPath, false, true );
-        dir.mkdir( filesPath );
+    // The naive implementation "delete info and files in every trash directory"
+    // breaks when deleted directories contain files owned by other users.
+    // We need to ensure that the .trashinfo file is only removed when the
+    // corresponding files could indeed be removed.
+
+    const TrashedFileInfoList fileInfoList = list();
+
+    TrashedFileInfoList::const_iterator it = fileInfoList.begin();
+    const TrashedFileInfoList::const_iterator end = fileInfoList.end();
+    for ( ; it != end ; ++it ) {
+        const TrashedFileInfo& info = *it;
+        const QString filesPath = info.physicalPath;
+        if ( synchronousDel( filesPath, true, true ) ) {
+            QFile::remove( infoPath( info.trashId, info.fileId ) );
+        } // else error code is set
     }
     fileRemoved();
+
+    return m_lastErrorCode == 0;
 }
 
 TrashImpl::TrashedFileInfoList TrashImpl::list()
--- branches/KDE/3.5/kdebase/kioslave/trash/trashimpl.h #583855:583856
@@ -72,7 +72,7 @@
     bool del( int trashId, const QString& fileId );
 
     /// Empty trash, i.e. delete all trashed files
-    void emptyTrash();
+    bool emptyTrash();
 
     /// Return true if the trash is empty
     bool isEmpty() const;

SVN commit 583941 by dfaure:

OK, u+w, not a+w, since a+w doesn't give us more chances of successful deletion of root-owned subdirs anyway.
CCBUG: 116371


 M  +1 -1      trashimpl.cpp  


--- branches/KDE/3.5/kdebase/kioslave/trash/trashimpl.cpp #583940:583941
@@ -499,7 +499,7 @@
         KFileItem fileItem( url, "inode/directory", KFileItem::Unknown );
         KFileItemList fileItemList;
         fileItemList.append( &fileItem );
-        KIO::ChmodJob* chmodJob = KIO::chmod( fileItemList, 0777, 0222, QString::null, QString::null, true /*recursive*/, false /*showProgressInfo*/ );
+        KIO::ChmodJob* chmodJob = KIO::chmod( fileItemList, 0200, 0200, QString::null, QString::null, true /*recursive*/, false /*showProgressInfo*/ );
         connect( chmodJob, SIGNAL( result(KIO::Job *) ),
                  this, SLOT( jobFinished(KIO::Job *) ) );
         qApp->eventLoop()->enterLoop();

Seems some people still have orphaned files in ~/.local/share/Trash/files:  https://bugs.launchpad.net/ubuntu/+source/meta-kde/+bug/30842, KDE bug 167051.
Let's discuss that in 167051.