Merge lp:~ted/url-dispatcher/intent-pkg-domain into lp:url-dispatcher/rtm-14.09

[Ted Gould]

Special handling for intent URLs

Makes it so that intent URLs are handled differently in that we use our current domain suffix for the package name instead of the domain. We don't want to choose packages based on domain there, as it isn't as important as the package that is supposed to be used. Many webapps think we're Android, so they send us intent URLs.

[Charles Kerr (charlesk)]

I found a few optional nitpicks, but LGTM

review: Approve

[Ted Gould (ted)]

Marked as Approved to get the new small changes in Charles' comments. And he's out sick so he can't do it himself.

=== modified file 'service/dispatcher.c'
--- service/dispatcher.c	2014-12-11 19:00:04 +0000
+++ service/dispatcher.c	2015-01-23 14:34:12 +0000
@@ -31,6 +31,7 @@
 static GRegex * applicationre = NULL;
 static GRegex * appidre = NULL;
 static GRegex * genericre = NULL;
+static GRegex * intentre = NULL;
 static sqlite3 * urldb = NULL;
 
 /* Errors */
@@ -326,6 +327,22 @@
 	return TRUE;
 }
 
+/* Determine the domain for an intent using the package variable */
+static gchar *
+intent_domain (const gchar * url)
+{
+	gchar * domain = NULL;
+	GMatchInfo * intentmatch = NULL;
+
+	if (g_regex_match(intentre, url, 0, &intentmatch)) {
+		domain = g_match_info_fetch(intentmatch, 1);
+
+		g_match_info_free(intentmatch);
+	}
+
+	return domain;
+}
+
 /* The core of the URL handling */
 gboolean
 dispatcher_url_to_appid (const gchar * url, gchar ** out_appid, const gchar ** out_url)
@@ -367,7 +384,14 @@
 	if (g_regex_match(genericre, url, 0, &genericmatch)) {
 		gboolean found = FALSE;
 		gchar * protocol = g_match_info_fetch(genericmatch, 1);
-		gchar * domain = g_match_info_fetch(genericmatch, 2);
+		gchar * domain = NULL;
+
+		/* We special case the intent domain (further comment there) */
+		if (g_strcmp0(protocol, "intent") == 0) {
+			domain = intent_domain(url);
+		} else {
+			domain = g_match_info_fetch(genericmatch, 2);
+		}
 
 		*out_appid = url_db_find_url(urldb, protocol, domain);
 		g_debug("Protocol '%s' for domain '%s' resulting in app id '%s'", protocol, domain, *out_appid);
@@ -450,6 +474,7 @@
 	applicationre = g_regex_new("^application:///([a-zA-Z0-9_\\.-]*)\\.desktop$", 0, 0, NULL);
 	appidre = g_regex_new("^appid://([a-z0-9\\.-]*)/([a-zA-Z0-9-]*)/([a-zA-Z0-9\\.-]*)$", 0, 0, NULL);
 	genericre = g_regex_new("^([a-z][a-z0-9]*):(?://(?:.*@)?([a-zA-Z0-9\\.-]*)(?::[0-9]*)?/?)?(.*)?$", 0, 0, NULL);
+	intentre = g_regex_new("^intent://.*package=([a-zA-Z0-9\\.]*);.*$", 0, 0, NULL);
 
 	g_bus_get(G_BUS_TYPE_SESSION, cancellable, bus_got, mainloop);
 
@@ -471,6 +496,7 @@
 	g_regex_unref(applicationre);
 	g_regex_unref(appidre);
 	g_regex_unref(genericre);
+	g_regex_unref(intentre);
 	sqlite3_close(urldb);
 
 	return TRUE;

=== modified file 'service/update-directory.c'
--- service/update-directory.c	2014-05-27 17:06:58 +0000
+++ service/update-directory.c	2015-01-23 14:34:12 +0000
@@ -55,6 +55,16 @@
 		return;
 	}
 
+	if (g_strcmp0(protocol, "intent") == 0) {
+		/* Special handling for intent, we have to have a domain suffix
+		   there because otherwise things will get crazy as we're handling
+		   it by package lookup in the service. */
+		if (suffix == NULL) {
+			g_warning("File %s: Array entry %d is an 'intent' protocol but doesn't have a package name", urldata->filename, index);
+			return;
+		}
+	}
+
 	if (!url_db_insert_url(urldata->db, urldata->filename, protocol, suffix)) {
 		const gchar * additional[7] = {
 			"Filename",

=== modified file 'tests/CMakeLists.txt'
--- tests/CMakeLists.txt	2014-09-22 15:20:40 +0000
+++ tests/CMakeLists.txt	2015-01-23 14:34:12 +0000
@@ -117,6 +117,7 @@
 add_definitions ( -DUPDATE_DIRECTORY_TOOL="${CMAKE_BINARY_DIR}/service/update-directory" )
 add_definitions ( -DUPDATE_DIRECTORY_URLS="${CMAKE_CURRENT_SOURCE_DIR}/test-urls-simple" )
 add_definitions ( -DUPDATE_DIRECTORY_VARIED="${CMAKE_CURRENT_SOURCE_DIR}/test-urls-varied" )
+add_definitions ( -DUPDATE_DIRECTORY_INTENT="${CMAKE_CURRENT_SOURCE_DIR}/test-urls-intent" )
 
 add_executable (directory-update-test directory-update-test.cc)
 target_link_libraries (directory-update-test

=== modified file 'tests/directory-update-test.cc'
--- tests/directory-update-test.cc	2014-01-13 20:46:10 +0000
+++ tests/directory-update-test.cc	2015-01-23 14:34:12 +0000
@@ -351,3 +351,25 @@
 	/* Cleanup */
 	sqlite3_close(db);
 }
+
+TEST_F(DirectoryUpdateTest, IntentTest)
+{
+	sqlite3 * db = url_db_create_database();
+
+	gchar * cmdline = g_strdup_printf("%s \"%s\"", UPDATE_DIRECTORY_TOOL, UPDATE_DIRECTORY_INTENT);
+	g_spawn_command_line_sync(cmdline, NULL, NULL, NULL, NULL);
+	g_free(cmdline);
+
+	EXPECT_EQ(3, get_file_count(db));
+	EXPECT_EQ(3, get_url_count(db));
+
+	EXPECT_TRUE(has_file(db, UPDATE_DIRECTORY_INTENT "/intent-single.url-dispatcher"));
+	EXPECT_TRUE(has_file(db, UPDATE_DIRECTORY_INTENT "/intent-mixed.url-dispatcher"));
+	EXPECT_TRUE(has_file(db, UPDATE_DIRECTORY_INTENT "/intent-no-good.url-dispatcher"));
+
+	EXPECT_TRUE(has_url(db, "intent", "intent.single"));
+	EXPECT_TRUE(has_url(db, "intent", "intent.mixed"));
+	EXPECT_TRUE(has_url(db, "intent", "intent.mixed.again"));
+
+	sqlite3_close(db);
+}

=== modified file 'tests/dispatcher-test.cc'
--- tests/dispatcher-test.cc	2014-12-11 18:33:34 +0000
+++ tests/dispatcher-test.cc	2015-01-23 14:34:12 +0000
@@ -55,6 +55,9 @@
 			url_db_set_file_motification_time(db, "/testdir/webapp.url-dispatcher", &timestamp);
 			url_db_insert_url(db, "/testdir/webapp.url-dispatcher", "http", "foo.com");
 
+			url_db_set_file_motification_time(db, "/testdir/intenter.url-dispatcher", &timestamp);
+			url_db_insert_url(db, "/testdir/intenter.url-dispatcher", "intent", "my.android.package");
+
 			sqlite3_close(db);
 
 			testbus = g_test_dbus_new(G_TEST_DBUS_NONE);
@@ -222,3 +225,25 @@
 	return;
 }
 
+TEST_F(DispatcherTest, IntentTest)
+{
+	gchar * out_appid = NULL;
+	const gchar * out_url = NULL;
+
+	/* Intent basic test */
+	EXPECT_TRUE(dispatcher_url_to_appid("intent://foo.google.com/maps#Intent;scheme=http;package=my.android.package;end", &out_appid, &out_url));
+	EXPECT_STREQ("intenter", out_appid);
+	g_free(out_appid);
+
+	/* Not our intent test */
+	out_appid = NULL;
+	EXPECT_FALSE(dispatcher_url_to_appid("intent://foo.google.com/maps#Intent;scheme=http;package=not.android.package;end", &out_appid, &out_url));
+	EXPECT_EQ(NULL, out_appid);
+
+	/* Ensure domain is ignored */
+	out_appid = NULL;
+	EXPECT_FALSE(dispatcher_url_to_appid("intent://my.android.package/maps#Intent;scheme=http;package=not.android.package;end", &out_appid, &out_url));
+	EXPECT_EQ(NULL, out_appid);
+
+	return;
+}

=== added directory 'tests/test-urls-intent'
=== added file 'tests/test-urls-intent/intent-mixed.url-dispatcher'
--- tests/test-urls-intent/intent-mixed.url-dispatcher	1970-01-01 00:00:00 +0000
+++ tests/test-urls-intent/intent-mixed.url-dispatcher	2015-01-23 14:34:12 +0000
@@ -0,0 +1,13 @@
+[
+	{
+		"protocol": "intent",
+		"domain-suffix": "intent.mixed"
+	},
+	{
+		"protocol": "intent"
+	},
+	{
+		"protocol": "intent",
+		"domain-suffix": "intent.mixed.again"
+	}
+]

=== added file 'tests/test-urls-intent/intent-no-good.url-dispatcher'
--- tests/test-urls-intent/intent-no-good.url-dispatcher	1970-01-01 00:00:00 +0000
+++ tests/test-urls-intent/intent-no-good.url-dispatcher	2015-01-23 14:34:12 +0000
@@ -0,0 +1,8 @@
+[
+	{
+		"protocol": "intent"
+	},
+	{
+		"protocol": "intent"
+	}
+]

=== added file 'tests/test-urls-intent/intent-single.url-dispatcher'
--- tests/test-urls-intent/intent-single.url-dispatcher	1970-01-01 00:00:00 +0000
+++ tests/test-urls-intent/intent-single.url-dispatcher	2015-01-23 14:34:12 +0000
@@ -0,0 +1,6 @@
+[
+	{
+		"protocol": "intent",
+		"domain-suffix": "intent.single"
+	}
+]