[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-panel-devel
Subject: Re: Review Request 127571: [taskmanager] Stop parsing executables as .desktop files
From: Rob Wu <rob () robwu ! nl>
Date: 2016-04-30 16:21:27
Message-ID: 20160430162127.11646.28518 () mimi ! kde ! org
[Download RAW message or body]
--===============7277151254497721357==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127571/
-----------------------------------------------------------
(Updated April 30, 2016, 6:21 p.m.)
Status
------
This change has been marked as submitted.
Review request for Plasma.
Changes
-------
Submitted with commit 71055e2b725fbb65e16055588f86a57c373d8b47 by Kai Uwe Broulik on \
behalf of Rob Wu to branch Plasma/5.6.
Repository: plasma-desktop
Description
-------
When a binary is launched via an absolute path, then launcherUrl is not
empty because of [1], even though it is not a .desktop file.
Consequently, when a user right-clicks on the task item, plasmashell
attempts to parse the executable as a .desktop configuration file.
Since this file is obviously not a .desktop file, parsing it as such
will fail, and KConfig floods ~/.xsession-errors with lots of errors.
This can quickly add hundreds of megabytes per right-click...
This patch resolves the problem by not constructing a KDesktopFile if
the launcherUrl is not a .desktop file.
An alternative (and more general) way to get rid of the symptoms is to
modify KDesktopFile / KConfig to stop parsing on the first error and
write the launcherUrl to the error log (or at the very least limit the
number of displayed errors). However, it can be argued that you should
not pass a non-.desktop file to KDesktopFile.
[1] https://quickgit.kde.org/?p=kde-workspace.git&a=commit&h=3a4b9c85fc21d14838ceac04bb0a70656ee7c701
Diffs
-----
applets/taskmanager/plugin/backend.cpp 07ddfbe
Diff: https://git.reviewboard.kde.org/r/127571/diff/
Testing
-------
The following steps demonstrate the issue, and confirms that the patch fixes the bug.
1. Create a simple GUI program, as follows:
```
// Compile: clang++ `pkg-config --cflags --libs Qt5Widgets` app.cpp -o app -g -fPIE
#include <QApplication>
#include <QLabel>
int main(int argc, char **argv) {
QApplication app(argc, argv);
QLabel label("Some GUI app");
label.show();
return app.exec();
}
```
2. Run the program via an absolute path: $PWD/app.sh
3. Right-click on the task item in the task bar (i.e. the program that you just \
launched). 4. Look at ~/.xsession-errors and observe that the following lines were \
added. >
"KConfigIni: In file /tmp/some-qt-app/app, line 1: " Invalid entry (missing '=')
"KConfigIni: In file /tmp/some-qt-app/app, line 2: " Invalid entry (missing '=')
"KConfigIni: In file /tmp/some-qt-app/app, line 3: " Invalid entry (missing '=')
"KConfigIni: In file /tmp/some-qt-app/app, line 4: " Invalid entry (missing '=')
"KConfigIni: In file /tmp/some-qt-app/app, line 5: " Invalid entry (missing ']')
"KConfigIni: In file /tmp/some-qt-app/app, line 6: " "Invalid escape sequence \
\"\\A\"." "KConfigIni: In file /tmp/some-qt-app/app, line 6: " "Invalid escape \
sequence \"\\\u0001\"."
... hundreds of similar lines
5. Compile plasma-desktop with this patch and restart plasmashell.
6. Repeat step 2 and 3.
7. Look at ~/.xsession-errors and observe that the errors are gone.
Thanks,
Rob Wu
--===============7277151254497721357==
MIME-Version: 1.0
Content-Type: text/html; charset="utf-8"
Content-Transfer-Encoding: 7bit
<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 \
solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;"> \
<tr> <td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/127571/">https://git.reviewboard.kde.org/r/127571/</a>
</td>
</tr>
</table>
<br />
<table bgcolor="#e0e0e0" width="100%" cellpadding="12" style="border: 1px gray solid; \
border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;"> <tr>
<td>
<h1 style="margin: 0; padding: 0; font-size: 10pt;">This change has been marked as \
submitted.</h1> </td>
</tr>
</table>
<br />
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: \
1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; \
-webkit-border-radius: 6px;"> <tr>
<td>
<div>Review request for Plasma.</div>
<div>By Rob Wu.</div>
<p style="color: grey;"><i>Updated April 30, 2016, 6:21 p.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Changes</h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: \
1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">Submitted with commit 71055e2b725fbb65e16055588f86a57c373d8b47 by Kai \
Uwe Broulik on behalf of Rob Wu to branch Plasma/5.6.</pre> </td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
plasma-desktop
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" \
style="border: 1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">When a binary is launched via an absolute path, then launcherUrl is not \
empty because of [1], even though it is not a .desktop file. Consequently, when a \
user right-clicks on the task item, plasmashell attempts to parse the executable as a \
.desktop configuration file. Since this file is obviously not a .desktop file, \
parsing it as such will fail, and KConfig floods ~/.xsession-errors with lots of \
errors. This can quickly add hundreds of megabytes per right-click...
This patch resolves the problem by not constructing a KDesktopFile if
the launcherUrl is not a .desktop file.
An alternative (and more general) way to get rid of the symptoms is to
modify KDesktopFile / KConfig to stop parsing on the first error and
write the launcherUrl to the error log (or at the very least limit the
number of displayed errors). However, it can be argued that you should
not pass a non-.desktop file to KDesktopFile.
[1] https://quickgit.kde.org/?p=kde-workspace.git&a=commit&h=3a4b9c85fc21d14838ceac04bb0a70656ee7c701</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: \
1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">The following steps demonstrate the issue, and \
confirms that the patch fixes the bug.</p> <ol style="padding: 0;text-rendering: \
inherit;margin: 0 0 0 2em;line-height: inherit;white-space: normal;"> <li \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
normal;">Create a simple GUI program, as follows:</li> </ol>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;"><div class="codehilite" style="background: \
#f8f8f8"><pre style="line-height: 125%"><span style="color: #408080; font-style: \
italic">// Compile: clang++ `pkg-config --cflags --libs Qt5Widgets` app.cpp -o app -g \
-fPIE</span> <span style="color: #BC7A00">#include <QApplication></span>
<span style="color: #BC7A00">#include <QLabel></span>
<span style="color: #B00040">int</span> <span style="color: \
#0000FF">main</span>(<span style="color: #B00040">int</span> argc, <span \
style="color: #B00040">char</span> <span style="color: #666666">**</span>argv) { \
QApplication app(argc, argv); QLabel label(<span style="color: #BA2121">"Some \
GUI app"</span>); label.show();
<span style="color: #008000; font-weight: bold">return</span> app.exec();
}
</pre></div>
</p>
<ol style="padding: 0;text-rendering: inherit;margin: 0 0 0 2em;line-height: \
inherit;white-space: normal;"> <li style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: normal;">Run the program via an absolute path: \
$PWD/app.sh</li> <li style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: normal;">Right-click on the task item in the task bar (i.e. the \
program that you just launched).</li> <li style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: normal;"> <p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Look \
at ~/.xsession-errors and observe that the following lines were added.</p> \
<blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid \
#bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;"> <p \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">"KConfigIni: In file /tmp/some-qt-app/app, line 1: " Invalid entry (missing \
'=') "KConfigIni: In file /tmp/some-qt-app/app, line 2: " Invalid entry (missing '=')
"KConfigIni: In file /tmp/some-qt-app/app, line 3: " Invalid entry (missing '=')
"KConfigIni: In file /tmp/some-qt-app/app, line 4: " Invalid entry (missing '=')
"KConfigIni: In file /tmp/some-qt-app/app, line 5: " Invalid entry (missing ']')
"KConfigIni: In file /tmp/some-qt-app/app, line 6: " "Invalid escape sequence \
\"\\A\"." "KConfigIni: In file /tmp/some-qt-app/app, line 6: " "Invalid escape \
sequence \"\\\u0001\"."
... hundreds of similar lines</p>
</blockquote>
</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: normal;"> <p style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: inherit;">Compile plasma-desktop with this patch \
and restart plasmashell.</p> </li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: normal;">Repeat step 2 and 3.</li> <li style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">Look \
at ~/.xsession-errors and observe that the errors are gone.</li> </ol></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>applets/taskmanager/plugin/backend.cpp <span style="color: \
grey">(07ddfbe)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/127571/diff/" style="margin-left: \
3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>
--===============7277151254497721357==--
[Attachment #3 (text/plain)]
_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic