[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: Eike Hein <hein () kde ! org>
Date: 2016-04-05 19:03:36
Message-ID: 20160405190336.15005.67936 () mimi ! kde ! org
[Download RAW message or body]
--===============1916243850473282321==
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/#review94310
-----------------------------------------------------------
Ship it!
Ship It!
- Eike Hein
On April 4, 2016, 1:44 p.m., Rob Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127571/
> -----------------------------------------------------------
>
> (Updated April 4, 2016, 1:44 p.m.)
>
>
> Review request for Plasma.
>
>
> 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
>
>
--===============1916243850473282321==
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 />
<p>
Ship it!
</p>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Ship It!</pre> <br />
<p>- Eike Hein</p>
<br />
<p>On April 4th, 2016, 1:44 p.m. UTC, Rob Wu wrote:</p>
<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 4, 2016, 1:44 p.m.</i></p>
<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>
--===============1916243850473282321==--
[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