TEMP: Add IndexFileForProcess, and functional testssam/index-mount-points

9 files changed, 209 insertions, 45 deletions

diff --git a/src/libtracker-miner/tracker-file-notifier.c b/src/libtracker-miner/tracker-file-notifier.c
index 49a0998f5..8b6213eea 100644
--- a/src/libtracker-miner/tracker-file-notifier.c
+++ b/src/libtracker-miner/tracker-file-notifier.c
@@ -1361,6 +1361,11 @@ indexing_tree_directory_removed (TrackerIndexingTree *indexing_tree,
 		tracker_crawler_stop (priv->crawler);
 		g_cancellable_cancel (priv->cancellable);
 
+		/* At some point between the 'if' and here, priv->current_index_root is
+		 * set to NULL! */
+		/* crawl_directories_start() or finish_current_directory() are the only
+		 * place it could really be happening ...
+		 */
 		root_data_free (priv->current_index_root);
 		priv->current_index_root = NULL;
 
diff --git a/src/libtracker-miner/tracker-miner-object.c b/src/libtracker-miner/tracker-miner-object.c
index 360ac7cd8..c6479fda1 100644
--- a/src/libtracker-miner/tracker-miner-object.c
+++ b/src/libtracker-miner/tracker-miner-object.c
@@ -1000,7 +1000,7 @@ miner_pause_internal (TrackerMiner  *miner,
 	}
 
 	if (calling_name) {
-		g_message ("Watching process with name:'%s'", calling_name);
+		g_message ("Pause: Watching process with name:'%s'", calling_name);
 		watch_name_id = g_bus_watch_name (TRACKER_IPC_BUS,
 		                                  calling_name,
 		                                  G_BUS_NAME_WATCHER_FLAGS_NONE,
diff --git a/src/miners/fs/tracker-miner-files-index.c b/src/miners/fs/tracker-miner-files-index.c
index 94323821d..a94ba2776 100644
--- a/src/miners/fs/tracker-miner-files-index.c
+++ b/src/miners/fs/tracker-miner-files-index.c
@@ -58,6 +58,7 @@ typedef struct {
 typedef struct {
 	TrackerMinerFS *miner;
 	GFile *file;
+	guint watch_id;
 } IndexFileForProcessData;
 
 typedef struct {
@@ -399,7 +400,7 @@ handle_method_call_index_file (TrackerMinerFilesIndex *miner,
 #endif /* REQUIRE_LOCATION_IN_CONFIG */
 
 	if (is_dir) {
-		tracker_miner_fs_check_directory (TRACKER_MINER_FS (priv->files_miner), file, do_checks);
+		tracker_miner_fs_check_directory (TRACKER_MINER_FS (priv->files_miner), file, do_checks, "tracker-IndexFile");
 	} else {
 		tracker_miner_fs_check_file (TRACKER_MINER_FS (priv->files_miner), file, do_checks);
 	}
@@ -410,6 +411,35 @@ handle_method_call_index_file (TrackerMinerFilesIndex *miner,
 	g_object_unref (file);
 }
 
+/* Since D-Bus provides an external interface for the miner process, we
+ * should be wary of rogue clients. The 'ownership' tracking in
+ * TrackerIndexingTree doesn't do any kind of sanity checking because it is
+ * internal API only. This function ensures that the application name on the
+ * bus is safe to use as an 'owner name' internally.
+ *
+ * In practice, the D-Bus server itself chooses the name and not the
+ * application, so this is unlikely to be a source of exploits anyway.
+ */
+static gboolean
+index_file_for_process_owner_name_is_safe (const char *owner_name) {
+	const int MAX_OWNER_NAME_LENGTH = 128;
+
+	/* This prevents attacks that cause Tracker to allocate lots of RAM. */
+	if (strlen (owner_name) > MAX_OWNER_NAME_LENGTH) {
+		g_warning ("In call to IndexFileForProcess: Owner name is longer than "
+		           "%i characters. Ignoring request.", MAX_OWNER_NAME_LENGTH);
+		return FALSE;
+	}
+
+	if (g_ascii_strncasecmp (owner_name, "tracker", strlen("tracker")) == 0) {
+		g_warning ("In call to IndexFileForProcess: Owner name '%s' uses "
+		           "reserved 'tracker' prefix. Ignoring request.", owner_name);
+		return FALSE;
+	}
+
+	return TRUE;
+}
+
 static void
 index_file_for_process_app_appears (GDBusConnection *connection,
                                     const gchar     *name,
@@ -421,6 +451,12 @@ index_file_for_process_app_appears (GDBusConnection *connection,
 	gboolean is_dir;
 	gboolean is_mount;
 
+	if (! index_file_for_process_owner_name_is_safe (name)) {
+		return;
+	}
+
+	g_debug ("Client app appeared: name %s, name owner %s", name, name_owner);
+
 	file_info = g_file_query_info (data->file,
 	                               G_FILE_ATTRIBUTE_STANDARD_TYPE,
 	                               G_FILE_QUERY_INFO_NOFOLLOW_SYMLINKS,
@@ -432,29 +468,48 @@ index_file_for_process_app_appears (GDBusConnection *connection,
 
 	if (is_mount) {
 		tracker_miner_fs_mount_add (TRACKER_MINER_FS (data->miner),
-		                            g_file_find_enclosing_mount (data->file, NULL, NULL));
+		                            g_file_find_enclosing_mount (data->file, NULL, NULL),
+		                            name);
 	} else if (is_dir) {
 		tracker_miner_fs_check_directory (TRACKER_MINER_FS (data->miner),
 		                                  data->file,
-		                                  FALSE);
+		                                  FALSE,
+		                                  name);
 	} else {
+		/* FIXME: there's no ownership tracking when we do this... but the
+		 * parent directory might still be enqueued for monitoring, so it's
+		 * a bit weird. Maybe this should be IndexDirectoryForProcess rather
+		 * than IndexFileForProcess ... ?
+		 */
 		tracker_miner_fs_check_file (TRACKER_MINER_FS (data->miner),
 		                             data->file,
 		                             FALSE);
 	}
-
-	index_file_for_process_data_destroy (data);
 }
 
 static void
 index_file_for_process_app_vanishes (GDBusConnection *connection,
-              const gchar     *name,
-              gpointer         user_data)
+                                     const gchar     *name,
+                                     gpointer         user_data)
 {
 	IndexFileForProcessData *data = user_data;
 
-	/* FIXME: What if the directory was already indexed? */
-	tracker_miner_fs_directory_remove (TRACKER_MINER_FS (data->miner), data->file);
+	g_debug ("Client app vanished: name %s", name);
+
+	/* When the app vanishes and was the only owner of a directory, the
+	 * directory is NOT removed from the store, but any indexing that was in
+	 * process is cancelled, and all monitors are removed.
+	 *
+	 * If the directory being watched was a removable device, tracker:available
+	 * will be set to false when the device disconnects.
+	 */
+	tracker_miner_fs_ignore (TRACKER_MINER_FS (data->miner), data->file, name);
+
+	/* Manually disable the watch and free the state struct. The app will need
+	 * to call IndexFileForProcess again if it crashed or something.
+	 */
+	g_bus_unwatch_name (data->watch_id);
+	index_file_for_process_data_destroy (data);
 }
 
 /* The IndexFileForProcess D-Bus method does the same as IndexFile, except that
@@ -477,11 +532,16 @@ handle_method_call_index_file_for_process (TrackerMinerFilesIndex *miner,
 
 	priv = TRACKER_MINER_FILES_INDEX_GET_PRIVATE (miner);
 
+	g_warning("CHUNT");
+
 	g_variant_get (parameters, "(&s)", &file_uri);
 	tracker_gdbus_async_return_if_fail (file_uri != NULL, invocation);
 
 	request = tracker_g_dbus_request_begin (invocation, "%s(uri:'%s')", __FUNCTION__, file_uri);
 
+	/* FIXME: this seems to return before adding the watch, without returning
+	 * an error ... need to debug, I guess.
+	 */
 	file = g_file_new_for_uri (file_uri);
 	file_info = g_file_query_info (file,
 	                               G_FILE_ATTRIBUTE_STANDARD_TYPE,
@@ -510,13 +570,14 @@ handle_method_call_index_file_for_process (TrackerMinerFilesIndex *miner,
 	 * index_file_for_process_app_appears(). This approach is race-free.
 	 */
 	sender = g_dbus_method_invocation_get_sender (invocation);
-	g_bus_watch_name (G_BUS_TYPE_SESSION,
-	                  sender,
-	                  G_BUS_NAME_WATCHER_FLAGS_NONE,
-	                  index_file_for_process_app_appears,
-	                  index_file_for_process_app_vanishes,
-	                  data,
-	                  index_file_for_process_data_destroy);
+	g_message ("IndexFileForProcess: Watching process with name: '%s'", sender);
+	data->watch_id = g_bus_watch_name (G_BUS_TYPE_SESSION,
+	                                   sender,
+	                                   G_BUS_NAME_WATCHER_FLAGS_NONE,
+	                                   index_file_for_process_app_appears,
+	                                   index_file_for_process_app_vanishes,
+	                                   data,
+	                                   NULL);
 
 	tracker_dbus_request_end (request, NULL);
 	g_dbus_method_invocation_return_value (invocation, NULL);
diff --git a/tests/functional-tests/302-miner-on-demand.py b/tests/functional-tests/302-miner-on-demand.py
index 6f6e6af39..be29e361b 100755
--- a/tests/functional-tests/302-miner-on-demand.py
+++ b/tests/functional-tests/302-miner-on-demand.py
@@ -18,41 +18,84 @@
 """
 Test on-demand indexing of locations.
 
-This feature exists so that applications can trigger the indexing of a
-removable device.
+This feature exists so that users can manually add files to the Tracker
+store, or trigger the indexing of a removable device.
 
-See: https://bugzilla.gnome.org/show_bug.cgi?id=680834
+Related bugs: https://bugzilla.gnome.org/show_bug.cgi?id=680834
 """
 
-from gi.repository import Gio
+from gi.repository import Gio, GLib
 
 import time
 import unittest2 as ut
 
 from common.utils.helpers import log
-from common.utils.minertest import CommonTrackerMinerTest, uri
+from common.utils.minertest import CommonTrackerMinerTest, DEFAULT_TEXT, path, uri
 
 
 class MinerOnDemandIndexingTest (CommonTrackerMinerTest):
 #    def test_01_index_file (self):
 #        """
-#        Indexing a file outside the configured indexing locations.
+#        Indexing and monitoring a file outside the configured locations.
 #
 #        This can also be done from the commandline with `tracker index FILE`.
 #        """
 #
+#        store = self.system.store
+#
 #        # This is created by CommonTrackerMinerTest.setup() from
 #        # common.utils.minertest module.
 #        unmonitored_file = 'test-no-monitored/file0.txt'
-#
 #        self.assertFileMissing(uri(unmonitored_file))
 #
 #        log("Queuing %s for indexing" % uri(unmonitored_file))
 #        self.system.miner_fs.index_iface.IndexFile(uri(unmonitored_file))
-#        self.system.store.await_resource_inserted('nfo:TextDocument',
-#                                                  url=uri(unmonitored_file))
+#        resource_id, resource_urn = store.await_resource_inserted(
+#            'nfo:TextDocument',
+#            url=uri(unmonitored_file),
+#            required_property='nie:plainTextContent')
+#        self.assertFileContents(resource_urn, DEFAULT_TEXT)
+#
+#        # When you pass a file to IndexFile, Tracker doesn't set up a monitor
+#        # so changes to the file are ignored. This is a bit inconsistent
+#        # compared to how directories are treated. The commented code will
+#        # not work, because of that.
+#
+#        #with open(path(unmonitored_file), 'w') as f:
+#        #    f.write('Version 2.0')
+#        #store.await_property_changed(resource_id, 'nie:plainTextContent')
+#        #self.assertFileContents(resource_urn, 'Version 2.0')
+#
+#    def test_02_index_directory (self):
+#        """
+#        Indexing and monitoring a directory outside the configured locations.
 #
-    def test_02_index_file_for_process(self):
+#        This can also be done from the commandline with `tracker index DIR`.
+#        """
+#
+#        store = self.system.store
+#
+#        # These are created by CommonTrackerMinerTest.setup() from
+#        # common.utils.minertest module.
+#        unmonitored_dir = 'test-no-monitored'
+#        unmonitored_file = 'test-no-monitored/file0.txt'
+#        self.assertFileMissing(uri(unmonitored_dir))
+#        self.assertFileMissing(uri(unmonitored_file))
+#
+#        log("Queuing %s for indexing" % uri(unmonitored_dir))
+#        self.system.miner_fs.index_iface.IndexFile(uri(unmonitored_dir))
+#        resource_id, resource_urn = store.await_resource_inserted(
+#            'nfo:TextDocument',
+#            url=uri(unmonitored_file),
+#            required_property='nie:plainTextContent')
+#        self.assertFileContents(resource_urn, DEFAULT_TEXT)
+#
+#        with open(path(unmonitored_file), 'w') as f:
+#            f.write('Version 2.0')
+#        store.await_property_changed(resource_id, 'nie:plainTextContent')
+#        self.assertFileContents(resource_urn, 'Version 2.0')
+
+   def test_04_index_directory_for_process(self):
         """
         Indexing a directory tree for a specific D-Bus name.
 
@@ -60,29 +103,65 @@ class MinerOnDemandIndexingTest (CommonTrackerMinerTest):
         that indexing of large removable devices can be tied to the lifetime of
         certain applications that let users 'opt in' to indexing a device.
         """
+
+        miner = self.system.miner_fs
+
+        unmonitored_dir = 'test-no-monitored/'
         unmonitored_file = 'test-no-monitored/file0.txt'
+        self.assertFileMissing(uri(unmonitored_dir))
         self.assertFileMissing(uri(unmonitored_file))
 
-        miner = self.system.miner_fs
+        # Open a separate connection to the session bus, so we can simulate the
+        # process ending again.
+        address = Gio.dbus_address_get_for_bus_sync(Gio.BusType.SESSION, None)
+        fake_app = Gio.DBusConnection.new_for_address_sync(
+            address,
+            Gio.DBusConnectionFlags.MESSAGE_BUS_CONNECTION |
+            Gio.DBusConnectionFlags.AUTHENTICATION_CLIENT,
+            None, None)
+        fake_app.init()
 
-        fake_app = Gio.bus_get_sync(Gio.BusType.SESSION)
         log("Opened D-Bus connection %s" % fake_app.get_unique_name())
 
+        # FIXME: there must be a better way of using GDBus in Python than
+        # this...
+
         # Index a file for the fake_app process, but then close the fake_app
         # straight away so the file doesn't get indexed. We do this while the
         # miner is paused, because otherwise the file might get indexed before
         # the app disappears, which would cause a spurious test failure.
-        cookie = miner.miner_fs.PauseForProcess(
-            fake_app.get_unique_name(),
-            "Avoid test process racing with miner process.")
-        miner.index_iface.IndexFile(uri(unmonitored_file))
-        fake_app.close()
-        log("Closed temporary D-Bus connection.")
-
+        DBUS_TIMEOUT = 5
+        pause_for_process_args = GLib.Variant('(ss)', (fake_app.get_unique_name(),
+            "Avoid test process racing with miner process."))
+        cookie = fake_app.call_sync(
+            'org.freedesktop.Tracker1.Miner.Files',
+            '/org/freedesktop/Tracker1/Miner/Files',
+            'org.freedesktop.Tracker1.Miner',
+            'PauseForProcess',
+            pause_for_process_args, None, 0, DBUS_TIMEOUT, None)
+
+        index_file_for_process_args = GLib.Variant('(s)', (uri(unmonitored_dir),))
+        fake_app.call_sync(
+            'org.freedesktop.Tracker1.Miner.Files.Index',
+            '/org/freedesktop/Tracker1/Miner/Files/Index',
+            'org.freedesktop.Tracker1.Miner.Files.Index',
+            'IndexFileForProcess',
+            index_file_for_process_args, None, 0, DBUS_TIMEOUT, None)
+
+        # This will cause Tracker to resume mining, and also stop indexing the
+        # file, possibly.
+        log("Closing temporary D-Bus connection.")
+        fake_app.close_sync()
+
+        bus2 = Gio.bus_get_sync(Gio.BusType.SESSION)
+        print bus2.call_sync('org.freedesktop.DBus', '/', 'org.freedesktop.DBus',
+                       'ListNames', None, None, 0, 1, None)
+
+        # We don't have to unpause the miner, it gets resumed 
         # The file should never get indexed, because the process disappeared.
-        miner.miner_fs.Resume(cookie)
-        time.sleep(5)
+        #miner.miner_fs.Resume(cookie)
         #self.assertFileMissing(uri(unmonitored_file))
+        # Currently this passes, when it should actually fail!
         self.system.store.await_resource_inserted('nfo:TextDocument',
                                                   url=uri(unmonitored_file))
         self.assertFilePresent(uri(unmonitored_file))
diff --git a/tests/functional-tests/common/utils/configuration.py.in b/tests/functional-tests/common/utils/configuration.py.in
index d6f544140..7eb886f74 100644
--- a/tests/functional-tests/common/utils/configuration.py.in
+++ b/tests/functional-tests/common/utils/configuration.py.in
@@ -30,6 +30,10 @@ MINERFS_BUSNAME = "org.freedesktop.Tracker1.Miner.Files"
 MINERFS_OBJ_PATH = "/org/freedesktop/Tracker1/Miner/Files"
 MINER_IFACE = "org.freedesktop.Tracker1.Miner"
 
+MINERFS_INDEX_BUSNAME = "org.freedesktop.Tracker1.Miner.Files.Index"
+MINERFS_INDEX_OBJ_PATH = "/org/freedesktop/Tracker1/Miner/Files/Index"
+MINER_FILES_INDEX_IFACE = "org.freedesktop.Tracker1.Miner.Files.Index"
+
 TRACKER_BACKUP_OBJ_PATH = "/org/freedesktop/Tracker1/Backup"                                            
 BACKUP_IFACE = "org.freedesktop.Tracker1.Backup"
 
diff --git a/tests/functional-tests/common/utils/helpers.py b/tests/functional-tests/common/utils/helpers.py
index 7b3ca88a9..4780ade3c 100644
--- a/tests/functional-tests/common/utils/helpers.py
+++ b/tests/functional-tests/common/utils/helpers.py
@@ -219,6 +219,10 @@ class Helper:
         self.abort_if_process_exits_with_status_0 = False
 
     def stop (self):
+        if options.is_manual_start():
+            log ("Manually stop %s" % self.PROCESS_NAME)
+            return
+
         start = time.time()
         if self.process.poll() == None:
             # It should step out of this loop when the miner disappear from the bus
@@ -235,10 +239,15 @@ class Helper:
                     self.process.wait()
 
         log ("[%s] stopped." % self.PROCESS_NAME)
+
         # Disconnect the signals of the next start we get duplicated messages
         self.bus._clean_up_signal_match (self.name_owner_match)
 
     def kill (self):
+        if options.is_manual_start():
+            log ("Manually kill %s" % self.PROCESS_NAME)
+            return
+
         self.process.kill ()
 
         # Name owner changed callback should take us out from this loop
@@ -653,7 +662,7 @@ class MinerFsHelper (Helper):
         index_bus_object = self.bus.get_object (cfg.MINERFS_INDEX_BUSNAME,
                                                 cfg.MINERFS_INDEX_OBJ_PATH)
         self.index_iface = dbus.Interface (index_bus_object,
-                                           dbus_interface = cfg.MINER_INDEX_IFACE)
+                                           dbus_interface = cfg.MINER_FILES_INDEX_IFACE)
 
     def stop (self):
         Helper.stop (self)
diff --git a/tests/functional-tests/common/utils/minertest.py b/tests/functional-tests/common/utils/minertest.py
index 1aa63ac1b..03aac88d3 100644
--- a/tests/functional-tests/common/utils/minertest.py
+++ b/tests/functional-tests/common/utils/minertest.py
@@ -45,13 +45,13 @@ CONF_OPTIONS = {
         'index-single-directories': GLib.Variant.new_strv([]),
         'index-optical-discs': GLib.Variant.new_boolean(False),
         'index-removable-devices': GLib.Variant.new_boolean(False),
-        'throttle': GLib.Variant.new_int32(5),
+        'throttle': GLib.Variant.new_int32(0),
+        'verbosity': GLib.Variant.new_string('detailed'),
     }
 }
 
 
 class CommonTrackerMinerTest (ut.TestCase):
-
     def prepare_directories (self):
         #
         #     ~/test-monitored/
@@ -130,3 +130,8 @@ class CommonTrackerMinerTest (ut.TestCase):
         if self.tracker.ask (query) == True:
             self.fail ("File <%s> should not be present in the database" %
                        file_url)
+
+    def assertFileContents(self, file_urn, expected_contents):
+        query = 'SELECT ?content { <%s> nie:plainTextContent ?content }' % file_urn
+        result = self.tracker.query(query)
+        self.assertEqual(result[0][0], expected_contents)
diff --git a/tests/functional-tests/common/utils/options.py b/tests/functional-tests/common/utils/options.py
index 6bc837905..4537e2db7 100644
--- a/tests/functional-tests/common/utils/options.py
+++ b/tests/functional-tests/common/utils/options.py
@@ -35,4 +35,5 @@ def is_manual_start ():
     """
     False to start the processes automatically
     """
+    return True
     return options.startmanually
diff --git a/utils/sandbox/tracker-sandbox.py b/utils/sandbox/tracker-sandbox.py
index 42d0e333a..1e51e5f25 100755
--- a/utils/sandbox/tracker-sandbox.py
+++ b/utils/sandbox/tracker-sandbox.py
@@ -320,7 +320,7 @@ def environment_set():
 		os.environ['TRACKER_LANGUAGE_STOPWORDS_DIR'] = os.path.join(opts.prefix, 'share', 'tracker', 'stop-words')
 
 	# Preferences
-	os.environ['TRACKER_USE_CONFIG_FILES'] = 'yes'
+	########os.environ['TRACKER_USE_CONFIG_FILES'] = 'yes'
 
 	#if opts.debug:
 	#	os.environ['TRACKER_USE_LOG_FILES'] = 'yes'
@@ -329,8 +329,8 @@ def environment_set():
 		os.environ['G_MESSAGES_DEBUG'] = 'all'
 		os.environ['TRACKER_VERBOSITY'] = '%d' % default_debug_verbosity
 		os.environ['DBUS_VERBOSE'] = '1'
-	else:
-		os.environ['TRACKER_VERBOSITY'] = '0'
+	#else:
+		#os.environ['TRACKER_VERBOSITY'] = '0'
 
 	debug('Using prefix location "%s"' % opts.prefix)
 	debug('Using index location "%s"' % index_location_abs)
@@ -491,7 +491,7 @@ if __name__ == "__main__":
 
 	# Set up environment variables and foo needed to get started.
 	environment_set()
-	config_set()
+	#config_set()
 
 	try:
 		if opts.update: