Merge lp:~3v1n0/unity/desktop-utilities-unescape-paths into lp:unity

[Marco Trevisan (Treviño)]

DesktopUtilities: unescape the desktop paths and ids before processing them

When computing the DesktopID or the file path from a DesktopID, we need to make sure that the processed file has been escaped.

Unit tests added.

[Michal Hruby (mhr3)]

Is this really what we want to do? We should properly distinguish methods that accept paths vs methods that work with URIs. Otherwise the next bug we'll get is "Unable to add desktop files with [insert special symbol here] in their name".

[Marco Trevisan (Treviño) (3v1n0)]

Mh, in effect this seems to happen now if a file name has contains a '%', while dragging it from dash... But at this point I would say that the lens should use the same format of the file managers and so always escaping these file URIs, or it won't be always possible to distinguish them.

[Daniel van Vugt (vanvugt)]

Note: This is already committed to lp:unity/6.0 before lp:unity!

[Michal Hruby (mhr3)]

I have my doubts about this, but let's at least keep trunk in sync with 6.0 and perhaps fix it up later.

review: Approve

=== modified file 'UnityCore/DesktopUtilities.cpp'
--- UnityCore/DesktopUtilities.cpp	2012-09-07 14:32:44 +0000
+++ UnityCore/DesktopUtilities.cpp	2012-10-02 21:09:22 +0000
@@ -87,6 +87,12 @@
   if (desktop_path.empty())
     return "";
 
+  glib::String unescaped(g_uri_unescape_string(desktop_path.c_str(), NULL));
+  std::string const& desktop_file = unescaped.Str();
+
+  if (desktop_file.empty())
+    return "";
+
   for (auto dir : default_paths)
   {
     if (!dir.empty())
@@ -100,11 +106,11 @@
         dir.append(G_DIR_SEPARATOR_S "applications" G_DIR_SEPARATOR_S);
       }
 
-      if (desktop_path.find(dir) == 0)
+      if (desktop_file.find(dir) == 0)
       {
         // if we are in a subdirectory of system path, the store name should
         // be subdir-filename.desktop
-        std::string desktop_suffix = desktop_path.substr(dir.size());
+        std::string desktop_suffix = desktop_file.substr(dir.size());
         std::replace(desktop_suffix.begin(), desktop_suffix.end(), G_DIR_SEPARATOR, '-');
 
         return desktop_suffix;
@@ -112,7 +118,7 @@
     }
   }
 
-  return desktop_path;
+  return desktop_file;
 }
 
 std::string DesktopUtilities::GetDesktopID(std::string const& desktop_path)
@@ -126,12 +132,18 @@
   if (desktop_id.empty())
     return "";
 
+  glib::String unescaped_id(g_uri_unescape_string(desktop_id.c_str(), NULL));
+  std::string const& id = unescaped_id.Str();
+
+  if (id.empty())
+    return "";
+
   glib::Object<GDesktopAppInfo> info;
 
-  if (desktop_id.find(G_DIR_SEPARATOR_S) != std::string::npos)
-    info = g_desktop_app_info_new_from_filename(desktop_id.c_str());
+  if (id.find(G_DIR_SEPARATOR_S) != std::string::npos)
+    info = g_desktop_app_info_new_from_filename(id.c_str());
   else
-    info = g_desktop_app_info_new(desktop_id.c_str());
+    info = g_desktop_app_info_new(id.c_str());
 
   if (info)
   {

=== modified file 'tests/test_desktop_utilities.cpp'
--- tests/test_desktop_utilities.cpp	2012-09-18 15:47:49 +0000
+++ tests/test_desktop_utilities.cpp	2012-10-02 21:09:22 +0000
@@ -36,9 +36,6 @@
 TEST(TestDesktopUtilitiesDesktopID, TestEmptyValues)
 {
   std::vector<std::string> empty_list;
-
-  EXPECT_THAT(DesktopUtilities::GetDesktopID(empty_list, "/some/path/to.desktop"),
-              Eq("/some/path/to.desktop"));
   EXPECT_THAT(DesktopUtilities::GetDesktopID(empty_list, "/some/path/to.desktop"),
               Eq("/some/path/to.desktop"));
 
@@ -77,6 +74,21 @@
               Eq("/some/path/applications/to.desktop"));
 }
 
+TEST(TestDesktopUtilitiesDesktopID, TestUnescapePath)
+{
+  std::vector<std::string> dirs;
+
+  dirs.push_back("/this/ path");
+  dirs.push_back("/that/path /");
+
+  EXPECT_THAT(DesktopUtilities::GetDesktopID(dirs, "/this/%20path/applications/to%20file.desktop"),
+              Eq("to file.desktop"));
+  EXPECT_THAT(DesktopUtilities::GetDesktopID(dirs, "/that/path%20/applications/to%20file.desktop"),
+              Eq("to file.desktop"));
+  EXPECT_THAT(DesktopUtilities::GetDesktopID(dirs, "/some/path/applications/to%20file.desktop"),
+              Eq("/some/path/applications/to file.desktop"));
+}
+
 TEST(TestDesktopUtilitiesDesktopID, TestSubdirectory)
 {
   std::vector<std::string> dirs;