diff options
-rw-r--r-- | src/libtracker-miner/tracker-file-notifier.c | 5 | ||||
-rw-r--r-- | src/libtracker-miner/tracker-miner-object.c | 2 | ||||
-rw-r--r-- | src/miners/fs/tracker-miner-files-index.c | 93 | ||||
-rwxr-xr-x | tests/functional-tests/302-miner-on-demand.py | 121 | ||||
-rw-r--r-- | tests/functional-tests/common/utils/configuration.py.in | 4 | ||||
-rw-r--r-- | tests/functional-tests/common/utils/helpers.py | 11 | ||||
-rw-r--r-- | tests/functional-tests/common/utils/minertest.py | 9 | ||||
-rw-r--r-- | tests/functional-tests/common/utils/options.py | 1 | ||||
-rwxr-xr-x | utils/sandbox/tracker-sandbox.py | 8 |
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: |