Merge lp:~saviq/qtubuntu/more-robust-desktop-parsing into lp:qtubuntu

[Michał Sawicz]

Make desktop file parsing more robust.

Ignore spaces around "=" and error out on malformed lines.

Unmerged revisions

158. By Michał Sawicz on 2013-07-25

Also skip if just the key was provided.

157. By Michał Sawicz on 2013-07-25

Ignore spaces around "=" in .desktop files.

156. By Michał Sawicz on 2013-07-25

Scan for a [Desktop Entry] group, don't assume it's on the first line.

=== modified file 'src/modules/application/application_manager.cc'
--- src/modules/application/application_manager.cc	2013-07-12 00:21:20 +0000
+++ src/modules/application/application_manager.cc	2013-07-25 13:00:32 +0000
@@ -181,16 +181,17 @@
        this, desktopFile.toLatin1().data());
   DASSERT(desktopFile != NULL);
   const struct { const char* const name; int size; unsigned int flag; } kEntryNames[] = {
-    { "Name=", sizeof("Name=") - 1, 1 << DesktopData::kNameIndex },
-    { "Comment=", sizeof("Comment=") - 1, 1 << DesktopData::kCommentIndex },
-    { "Icon=", sizeof("Icon=") - 1, 1 << DesktopData::kIconIndex },
-    { "Exec=", sizeof("Exec=") - 1, 1 << DesktopData::kExecIndex },
-    { "X-Ubuntu-StageHint=", sizeof("X-Ubuntu-StageHint=") - 1, 1 << DesktopData::kStageHintIndex }
+    { "Name", sizeof("Name") - 1, 1 << DesktopData::kNameIndex },
+    { "Comment", sizeof("Comment") - 1, 1 << DesktopData::kCommentIndex },
+    { "Icon", sizeof("Icon") - 1, 1 << DesktopData::kIconIndex },
+    { "Exec", sizeof("Exec") - 1, 1 << DesktopData::kExecIndex },
+    { "Path", sizeof("Path") - 1, 1 << DesktopData::kPathIndex },
+    { "X-Ubuntu-StageHint", sizeof("X-Ubuntu-StageHint") - 1, 1 << DesktopData::kStageHintIndex }
   };
   const unsigned int kAllEntriesMask =
       (1 << DesktopData::kNameIndex) | (1 << DesktopData::kCommentIndex)
       | (1 << DesktopData::kIconIndex) | (1 << DesktopData::kExecIndex)
-      | (1 << DesktopData::kStageHintIndex);
+      | (1 << DesktopData::kPathIndex) | (1 << DesktopData::kStageHintIndex);
   const unsigned int kMandatoryEntriesMask =
       (1 << DesktopData::kNameIndex) | (1 << DesktopData::kIconIndex)
       | (1 << DesktopData::kExecIndex);
@@ -206,11 +207,9 @@
   }
 
   // Validate "magic key" (standard group header).
-  if (file.readLine(buffer, kBufferSize) != -1) {
-    if (strncmp(buffer, "[Desktop Entry]", sizeof("[Desktop Entry]" - 1))) {
-      DLOG("not a desktop file");
-      return false;
-    }
+  while (file.readLine(buffer, kBufferSize) != -1) {
+    if (!strncmp(buffer, "[Desktop Entry]", sizeof("[Desktop Entry]" - 1)))
+      break;
   }
 
   int length;
@@ -223,17 +222,50 @@
         DLOG("reached next group header, leaving loop");
         break;
       }
+      bool validLine = true;
       // Lookup entries ignoring duplicates if any.
       for (int i = 0; i < kEntriesCount; i++) {
         if (!strncmp(buffer, kEntryNames[i].name, kEntryNames[i].size)) {
           if (~entryFlags & kEntryNames[i].flag) {
+            int keyLength = kEntryNames[i].size;
             buffer[length-1] = '\0';
-            entries_[i] = QString::fromLatin1(&buffer[kEntryNames[i].size]);
+
+            if (length == keyLength + 1) {
+                validLine = false;
+                break;
+            }
+
+            if ((const char)buffer[keyLength] != ' ' && (const char)buffer[keyLength] != '=') {
+              // Skip, not the key we're looking for.
+              continue;
+            }
+
+            bool gotEqual = false;
+            while(keyLength < length) {
+              if ((const char)buffer[keyLength] ==  ' ') {
+                // Ignore spaces.
+              } else if ((const char)buffer[keyLength] == '=') {
+                gotEqual = true;
+              } else {
+                break;
+              }
+              keyLength++;
+            }
+            if (!gotEqual) {
+              validLine = false;
+              break;
+            }
+            entries_[i] = QString::fromLatin1(&buffer[keyLength]);
             entryFlags |= kEntryNames[i].flag;
+            DLOG("Found entry for %s: %s", kEntryNames[i].name, entries_[i].toLatin1().data());
             break;
           }
         }
       }
+      if (!validLine) {
+        DLOG("Malformed .desktop file entry, ignoring: %s", buffer);
+        continue;
+      }
       // Stop when matching the right number of entries.
       if (entryFlags == kAllEntriesMask) {
         break;
@@ -243,11 +275,12 @@
 
   // Check that the mandatory entries are set.
   if ((entryFlags & kMandatoryEntriesMask) == kMandatoryEntriesMask) {
-    DLOG("loaded desktop file with name='%s', comment='%s', icon='%s', exec='%s', stagehint='%s'",
+    DLOG("loaded desktop file with name='%s', comment='%s', icon='%s', exec='%s', path='%s', stagehint='%s'",
          entries_[DesktopData::kNameIndex].toLatin1().data(),
          entries_[DesktopData::kCommentIndex].toLatin1().data(),
          entries_[DesktopData::kIconIndex].toLatin1().data(),
          entries_[DesktopData::kExecIndex].toLatin1().data(),
+         entries_[DesktopData::kPathIndex].toLatin1().data(),
          entries_[DesktopData::kStageHintIndex].toLatin1().data());
     return true;
   } else {
@@ -603,11 +636,19 @@
   // Start process.
   bool result;
   qint64 pid = 0;
-  struct passwd* passwd = getpwuid(getuid());
-  DLOG("current working directory: '%s'", passwd ? passwd->pw_dir : "/");
+  QString path = "/";
+  // respect Path from .desktop file
+  if (desktopData->path() != "") {
+    path = desktopData->path();
+  } else {
+    struct passwd* passwd = getpwuid(getuid());
+    if (passwd)
+      path = passwd->pw_dir;
+  }
+  DLOG("current working directory: '%s'", path.toLatin1().data());
   QByteArray envSetAppId = QString("APP_ID=%1").arg(appId).toLocal8Bit();
   putenv(envSetAppId.data()); // envSetAppId must be available and unmodified until the env var is unset
-  result = QProcess::startDetached(exec, arguments, QString(passwd ? passwd->pw_dir : "/"), &pid);
+  result = QProcess::startDetached(exec, arguments, path, &pid);
   QByteArray envClearAppId = QString("APP_ID").toLocal8Bit();
   putenv(envClearAppId.data()); // now it's safe to deallocate envSetAppId.
   DLOG_IF(result == false, "process failed to start");

=== modified file 'src/modules/application/application_manager.h'
--- src/modules/application/application_manager.h	2013-05-29 15:50:52 +0000
+++ src/modules/application/application_manager.h	2013-07-25 13:00:32 +0000
@@ -44,6 +44,7 @@
   QString comment() const { return entries_[kCommentIndex]; }
   QString icon() const { return entries_[kIconIndex]; }
   QString exec() const { return entries_[kExecIndex]; }
+  QString path() const { return entries_[kPathIndex]; }
   QString stageHint() const { return entries_[kStageHintIndex]; }
   bool loaded() const { return loaded_; }
 
@@ -52,8 +53,9 @@
     kCommentIndex = 1,
     kIconIndex = 2,
     kExecIndex = 3,
-    kStageHintIndex = 4,
-    kNumberOfEntries = 5;
+    kPathIndex = 4,
+    kStageHintIndex = 5,
+    kNumberOfEntries = 6;
 
   bool loadDesktopFile(QString desktopFile);