Bug 39537

This is *really* noticable when using a slow-loading editor (like OpenOffice) 
when opening large Word DOC files ( PHBs never learn ;). Normally my policy 
would be to launch an attachment, and go ahead to the next unread (in my case 
with the + rather than Space).

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

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

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

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

Why weren't the comments found in the duplicate bugs included as part of this bug?

FWIW, this behaviour still exists in KMail 1.5.4. Does anyone know if this issue will be resolved in the upcoming KDE 3.2 release?

There's no need to duplicate the comments of the duplicates because the duplicates are linked to this bug report.

And, no, it won't be resolved in KDE 3.2. Otherwise we would have closed this bug report already.

I want to fix this, using a list of kurls instead of QStringList mTempFiles [kmreaderwin] and toggling autodelete, but instead of the attach path being {...}/kmailxxxxx.y/filename, the attachment would be temporarily stored at {...}/kmail_xxx.$attID.$attname would that be acceptable?
That means the title in a word processor would be a bit ugly.
btw that's how ms does it.

sorry, that should be KTempFile

Created attachment 7293 [details]
use hard links and KRun's tempfile flags to launch attachments

Bug 39537, is about having to wait till the attachment is opened, before moving
to the next email.
This patch creates a hard link to the atm file, then uses the KRun's tempfile
flag to launch attachment.
Attachments launched will have names like ${atmName}_[xxxxxx].${extension}.

could someone review it?

You need to remove KMReaderWin:: from the .h file.

+  link(mAtmCurrentName.latin1(), linkName.latin1());
Should be QFile::encodeName()

+  return (linkName);
Looks like a method call :) We usually don't use () there :)

+ QFile( url.path() ).remove();
QFile::remove( url.path() ) might be a bit faster

Someone needs to check if kdelibs-3.2 had the "tempfile" flag. If not,
then the patch can't be backported to KDE_3_3_BRANCH. In any case
it's no problem for HEAD.

Created attachment 7366 [details]
use links and KRun's tempfile flag to launch attachments

From the webcvs I see tempfile flags in KRun::run &&
KRun::displayOpenWithDialog in KDE_3_2_BRANCH.
I've fixed the issues raised by David.
Lets see, the patch is 32 lines of code, David found 4 problems, so we have an
error every 8 lines.
I'll add more comments next time, to achieve a lower error percentage ;-)

----------  Forwarded Message  ----------

Subject: Re: Fwd: [Bug 39537] attachments' temporary files are deleted too soon
Date: Monday 06 September 2004 05:18
From: George Staikos <staikos@kde.org>
To: David Faure <faure@kde.org>, security@kde.org


Actually it's quite a big race condition since the filename is known.  It's 
even easier to exploit than normal.  A script could just suck up cpu and 
constantly try to create files(links) with names that already exist.  It's 
probably sufficient to setAutoDelete(false);

On Wednesday 01 September 2004 07:46, you wrote:
> A little question for people with more security knowledge than me:
>
> Using the name of a just-deleted KTempFile is a small race condition,
> right? Any advice on how to do this better?
>
> +QString KMReaderWin::createTempAtmFile() const
> +{
> +  QFileInfo atmFileInfo(mAtmCurrentName);
> +  QString linkName;
> +
> +  KTempFile *linkFile = new KTempFile( locateLocal("tmp",
> atmFileInfo.fileName() +"_["), +                            "]."+
> atmFileInfo.extension() );
> +  linkFile->setAutoDelete(true);
> +  linkName = linkFile->name();
> +  delete linkFile;
> +
> +  link(QFile::encodeName(mAtmCurrentName), QFile::encodeName(linkName));
> +  //kdDebug(5004) << "link:" << mAtmCurrentName << " to " << 
> linkName.latin1() << endl; +  //kdDebug(5004) << "URL:" << url << " " <<
> mAtmCurrentName << endl; +  return linkName;
> +
> +}
>
>
> ----------  Forwarded Message  ----------
>
> Subject: [Bug 39537] attachments' temporary files are deleted too soon
> Date: Tuesday 31 August 2004 10:01
> From: earl grey <earlgreykde@bigpond.com>
> To: kmail-devel@kde.org
>
> ------- Additional Comments From earlgreykde bigpond com  2004-08-31 10:01
> ------- Created an attachment (id=7366)
>  --> (http://bugs.kde.org/attachment.cgi?id=7366&action=view)
> use links and KRun's tempfile flag to launch attachments
>
> -------------------------------------------------------

On Thursday 09 September 2004 21:25, Waldo Bastian wrote:
> On Wednesday 01 September 2004 13:46, David Faure wrote:
> > A little question for people with more security knowledge than me:
> >
> > Using the name of a just-deleted KTempFile is a small race condition,
> > right?
> 
> I don't think so, link will fail if the file already exists.... you may want 
> to check the return value though.

Yes (and try again with another tempfilename on EEXIST, right?)

> > Any advice on how to do this better? 
> 
> What are you trying to do? Is mAtmCurrentName located in "tmp" as well? 
> Otherwise you may not be able to link.

AFAIU this is about hardlinking to a short-lived tempfile in order to extend 
its lifetime for the duration of the launched application (that second
"tempfile" is then deleted by KRun, when the application exits).
I'll let the person who made the patch comment if I'm wrong, I'm just
the middle man here :)

> Yes (and try again with another tempfilename on EEXIST, right?)

I would just fail the whole operation in that case. It's very unlikely that you run into this situation because only the user is supposed to have write access to the directory in the first place.

Created attachment 7506 [details]
use hard links and KRun's tempfile flags to launch attachments

Added a check to the link call and retry up to 10 times.
If it still can't create the link, then it uses the original file.
This all happens in the /tmp kstandarddir, which has perm 700, so security
shouldn't be a concern, and hardlinks should work.

CVS commit by faure: 

Apply fix for longstanding bug "attachments' temporary files are deleted too soon",
with thanks to "earl grey" for his patch (and patience :})
The KRun "delete tempfile after use" feature comes in handy for this (BTW I checked
and kdelibs-3.2 had it already).
Slightly altered the final patch to remove the loop as advised by Waldo.
CCMAIL: 39537-done@bugs.kde.org


  M +43 -5     kmreaderwin.cpp   1.777.2.2
  M +1 -0      kmreaderwin.h   1.197.2.1


--- kdepim/kmail/kmreaderwin.cpp  #1.777.2.1:1.777.2.2
@@ -1878,9 +1878,19 @@ void KMReaderWin::slotDoAtmOpen()
   }
 
-  KURL url;
-  url.setPath( mAtmCurrentName );
   KURL::List lst;
+  KURL url;
+  bool autoDelete = true;
+  QString fname = createAtmFileLink(); 
+
+  if ( fname == QString::null ) {
+    autoDelete = false;
+    fname = mAtmCurrentName;
+  }
+
+  url.setPath( fname );
   lst.append( url );
-  KRun::run( *mOffer, lst );
+  if ( (KRun::run( *mOffer, lst, autoDelete ) <= 0) && autoDelete ) {
+      QFile::remove(url.path());
+  }
 }
 
@@ -1893,7 +1903,17 @@ void KMReaderWin::slotAtmOpenWith()
     KURL::List lst;
     KURL url;
-    url.setPath(mAtmCurrentName);
+    bool autoDelete = true;
+    QString fname = createAtmFileLink(); 
+
+    if ( fname == QString::null ) {
+      autoDelete = false;
+      fname = mAtmCurrentName;
+    }
+
+    url.setPath( fname );
     lst.append(url);
-    KRun::displayOpenWithDialog(lst);
+    if ( (! KRun::displayOpenWithDialog(lst, autoDelete)) && autoDelete ) {
+      QFile::remove(url.path());
+    }
 }
 
@@ -2191,4 +2211,22 @@ void KMReaderWin::slotIMChat()
 
 //-----------------------------------------------------------------------------
+QString KMReaderWin::createAtmFileLink() const
+{
+  QFileInfo atmFileInfo(mAtmCurrentName);
+
+  KTempFile *linkFile = new KTempFile( locateLocal("tmp", atmFileInfo.fileName() +"_["),
+                          "]."+ atmFileInfo.extension() );
+
+  linkFile->setAutoDelete(true);
+  QString linkName = linkFile->name();
+  delete linkFile;
+
+  if ( link(QFile::encodeName(mAtmCurrentName), QFile::encodeName(linkName)) == 0 ) {
+    return linkName; // success
+  }
+  kdWarning() << "Couldn't link to " << mAtmCurrentName << endl;
+  return QString::null;
+}
+
 #include "kmreaderwin.moc"
 

--- kdepim/kmail/kmreaderwin.h  #1.197:1.197.2.1
@@ -388,4 +388,5 @@ private:
   void createActions( KActionCollection * ac );
   void saveSplitterSizes( KConfigBase & c ) const;
+  QString createAtmFileLink() const;
 
 private:

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

Fix reverted in KDE_3_3_BRANCH, since it breaks with apps that go into the background
like ark, kfmclient or korganizer. Proper fix will be done in HEAD tomorrow.

Has this already been fixed? Still got the problem in KDE 3.3.1.

No, and if it was reverted in BRANCH, it will not be fixed until 3.4, I guess. This means some more months without this fix.

Yep. Meanwhile you just have to make sure to wait until the file is opened, before switching to another mail.
Not really critical once you know this is how to make it work :)

> Yep. Meanwhile you just have to make sure to wait
> until the file is opened, before switching to another mail.

This is actually not always true. In case of files that are opened by konqueror, e.g. *.xhtml attachments you will have to make sure that konqueror is already open, otherwise it won't startup quick enough even if you did not switch to another mail.

> ------- Additional Comments From sven.burmeister gmx net  2004-11-08 18:35 -------
> > Yep. Meanwhile you just have to make sure to wait
> > until the file is opened, before switching to another mail.
> 
> This is actually not always true. In case of files that are opened by konqueror, e.g. *.xhtml attachments you will have to make sure that konqueror is already open, otherwise it won't startup quick enough even if you did not switch to another mail.

That's with the guilty patch, not without it. This is why I reverted it, in the branch.

Is this bug really fixed in KDE 3.3.2 ?
I use Debian Sid with KDE 3.3.2 and KMail 1.7.2, and the bug still occurs when changing to another mail when there is opening a attached file. The temporary file is deleted and the external program can't find it.
Why is this bug "FIXED"?

> Is this bug really fixed in KDE 3.3.2 ?

I am not sure about 3.3.2, it never happened to me in 3.4 beta1 or 2, so I 
think it is fixed, yet not in 3_3_BRANCH.

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

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

This bug is present in kde 3.5.6 (kmail 1.9.6).
kmail should wait() for the viewer application to exit before deleting the temp file.

Hi. i'm linking this bug with ubuntu's launchpad corresponding bug

https://bugs.launchpad.net/kdepim/+bug/70981

I hope this can be addressed soon.

Thanks a lot for your help 

Git commit e8319587df9862b0f6f5ea18114058dcf84194c4 by Halla Rempt.
Committed on 11/04/2022 at 09:38.
Pushed by rempt into branch 'rempt/work/bug_39537'.

Truncate the log file if it's bigger than 100mb

Maybe we should make a back-up, though?

M  +9    -1    libs/global/KisUsageLogger.cpp

https://invent.kde.org/graphics/krita/commit/e8319587df9862b0f6f5ea18114058dcf84194c4

A possibly relevant merge request was started @ https://invent.kde.org/graphics/krita/-/merge_requests/1412