Merge lp:~ted/url-dispatcher/more-sophisticated-urls into lp:url-dispatcher/rtm-14.09

[Ted Gould]

Making generic regular expression more robust

Turns out some people need some more flexibility in our URLs. Let's give it to them.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:81
http://jenkins.qa.ubuntu.com/job/url-dispatcher-ci/149/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/url-dispatcher-utopic-amd64-ci/43
    SUCCESS: http://jenkins.qa.ubuntu.com/job/url-dispatcher-utopic-armhf-ci/41
    SUCCESS: http://jenkins.qa.ubuntu.com/job/url-dispatcher-utopic-i386-ci/41

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/url-dispatcher-ci/149/rebuild

review: Approve (continuous-integration)

[Olivier Tilloy (osomon)]

Not sure about magnet links, but for DialerTest, shouldn’t we also test that a URL of the form "tel:///[phone]" (i.e. with slashes) is valid? I don’t know how frequent this kind of link is around the web, but given that both forms are theoretically valid, we should assume we’ll encounter both.

[Charles Kerr (charlesk)]

The patch LGTM.

I don't know how common tel:/// is and at first glance it doesn't seem to conform to RFC 3966... unless there's at least some anecdotal evidence that tel:/// is frequent in the wild, I'd vote against it.

Ted, looks like this MP predates the branching your scripts did last week. Does this need to be submitted against /14.10 as well?

review: Approve

[Charles Kerr (charlesk)]

review: Approve

[Olivier Tilloy (osomon)]

It seems that "tel:///" is not correct indeed. Although I couldn’t find proper anecdotal evidence, this question on stackoverflow (http://stackoverflow.com/questions/22704662/using-slashes-in-tel-uris) seems to imply that there are tel links in the wild with two slashes (which is definitely incorrect).

As I understand it, the generic RE is used for matching "http" URLs as well as "tel", so unless we introduce a more specific RE for matching "tel" URLs, the generic RE will accept "tel" links with slashes, right?

I don’t think that’s a big deal though, the issue at hand (handling "tel:" links) is fixed, the logic can probably be refined later with a lower priority.

=== modified file 'service/dispatcher.c'
--- service/dispatcher.c	2014-09-22 14:46:06 +0000
+++ service/dispatcher.c	2014-10-29 14:48:52 +0000
@@ -370,6 +370,7 @@
 		gchar * 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);
 
 		if (*out_appid != NULL) {
 			found = TRUE;
@@ -448,7 +449,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-z0-9\\.-]*)?/?(.*)?$", 0, 0, NULL);
+	genericre = g_regex_new("^([a-z][a-z0-9]*):(?://(?:.*@)(a-z0-9\\.-]*)(?::[0-9])/)?(.*)?$", 0, 0, NULL);
 
 	g_bus_get(G_BUS_TYPE_SESSION, cancellable, bus_got, mainloop);
 

=== modified file 'tests/dispatcher-test.cc'
--- tests/dispatcher-test.cc	2014-09-22 14:46:06 +0000
+++ tests/dispatcher-test.cc	2014-10-29 14:48:52 +0000
@@ -43,6 +43,12 @@
 			url_db_set_file_motification_time(db, "/testdir/com.ubuntu.calendar_calendar_9.8.2343.url-dispatcher", &timestamp);
 			url_db_insert_url(db, "/testdir/com.ubuntu.calendar_calendar_9.8.2343.url-dispatcher", "calendar", NULL);
 
+			url_db_set_file_motification_time(db, "/testdir/com.ubuntu.dialer_dialer_1234.url-dispatcher", &timestamp);
+			url_db_insert_url(db, "/testdir/com.ubuntu.dialer_dialer_1234.url-dispatcher", "tel", NULL);
+
+			url_db_set_file_motification_time(db, "/testdir/magnet-test.url-dispatcher", &timestamp);
+			url_db_insert_url(db, "/testdir/magnet-test.url-dispatcher", "magnet", NULL);
+
 			sqlite3_close(db);
 
 			testbus = g_test_dbus_new(G_TEST_DBUS_NONE);
@@ -152,3 +158,38 @@
 
 	return;
 }
+
+TEST_F(DispatcherTest, DialerTest)
+{
+	gchar * out_appid = NULL;
+	const gchar * out_url = NULL;
+
+	/* Base Telephone */
+	EXPECT_TRUE(dispatcher_url_to_appid("tel:[phone]", &out_appid, &out_url));
+	EXPECT_STREQ("com.ubuntu.dialer_dialer_1234", out_appid);
+	g_free(out_appid);
+
+	/* Tel with bunch of commas */
+	EXPECT_TRUE(dispatcher_url_to_appid("tel:911,,,1,,1,,2", &out_appid, &out_url));
+	EXPECT_STREQ("com.ubuntu.dialer_dialer_1234", out_appid);
+	g_free(out_appid);
+
+	/* Telephone with slashes */
+	EXPECT_TRUE(dispatcher_url_to_appid("tel:///[phone]", &out_appid, &out_url));
+	EXPECT_STREQ("com.ubuntu.dialer_dialer_1234", out_appid);
+	g_free(out_appid);
+
+	return;
+}
+
+TEST_F(DispatcherTest, MagnetTest)
+{
+	gchar * out_appid = NULL;
+	const gchar * out_url = NULL;
+
+	EXPECT_TRUE(dispatcher_url_to_appid("magnet:?xt=urn:ed2k:31D6CFE0D16AE931B73C59D7E0C089C0&xl=0&dn=zero_len.fil&xt=urn:bitprint:3I42H3S6NNFQ2MSVX7XZKYAYSCX5QBYJ.LWPNACQDBZRYXW3VHJVCJ64QBZNGHOHHHZWCLNQ&xt=urn:md5:D41D8CD98F00B204E9800998ECF8427E", &out_appid, &out_url));
+	EXPECT_STREQ("magnet-test", out_appid);
+	g_free(out_appid);
+
+	return;
+}