Ubuntu Application Launcher

Merge lp:~ted/ubuntu-app-launch/uri-splitting into lp:ubuntu-app-launch/14.04

  1. uri-splitting
  2. Merge into trunk.14.04
Proposed by Ted Gould
Status: Merged
Approved by: Charles Kerr
Approved revision: 92
Merged at revision: 86
Proposed branch: lp:~ted/ubuntu-app-launch/uri-splitting
Merge into: lp:ubuntu-app-launch/14.04
Diff against target: 387 lines (+94/-152)
2 files modified
helpers.c (+75/-127)
tests/helper-test.cc (+19/-25)
To merge this branch: bzr merge lp:~ted/ubuntu-app-launch/uri-splitting
Reviewer Review Type Date Requested Status
Charles Kerr (community) Approve
PS Jenkins bot (community) continuous-integration Approve
Review via email: mp+196316@code.launchpad.net

Commit message

Ensure quoted single URIs passed to a %U are unquoted

Description of the change

Fixes the problem (see test) but also takes the time to ensure that all URIs are separate arguments and fixes the tests in response to that. Overall this makes us cleaner and gets rid of some crufty code.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:92
http://jenkins.qa.ubuntu.com/job/upstart-app-launch-ci/157/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/upstart-app-launch-trusty-amd64-ci/16
    SUCCESS: http://jenkins.qa.ubuntu.com/job/upstart-app-launch-trusty-armhf-ci/16
    SUCCESS: http://jenkins.qa.ubuntu.com/job/upstart-app-launch-trusty-i386-ci/16

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/upstart-app-launch-ci/157/rebuild

review: Approve (continuous-integration)
Revision history for this message
Charles Kerr (charlesk) wrote :

Looks good!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'helpers.c'
2--- helpers.c 2013-11-18 21:20:41 +0000
3+++ helpers.c 2013-11-22 16:42:53 +0000
4@@ -273,121 +273,61 @@
5 return retval;
6 }
7
8-/* free a string in an array */
9+/* Put the list of files into the argument array */
10+static inline void
11+file_list_handling (GArray * outarray, gchar ** list, gchar * (*dup_func) (const gchar * in))
12+{
13+ /* No URLs, cool, this is a noop */
14+ if (list == NULL || list[0] == NULL) {
15+ return;
16+ }
17+
18+ int i;
19+ for (i = 0; list[i] != NULL; i++) {
20+ gchar * entry = dup_func(list[i]);
21+
22+ /* No NULLs */
23+ if (entry != NULL && entry[0] != '\0') {
24+ g_array_append_val(outarray, entry);
25+ } else {
26+ g_free(entry);
27+ }
28+ }
29+}
30+
31+/* Parse a desktop exec line and return the next string */
32 static void
33-free_string (gpointer value)
34-{
35- gchar ** str = (gchar **)value;
36- g_free(*str);
37- return;
38-}
39-
40-/* Builds the file list from the URI list */
41-static gchar *
42-build_file_list (const gchar * uri_list)
43-{
44- gchar ** uri_split = NULL;
45- if (!g_shell_parse_argv(uri_list, NULL, &uri_split, NULL)) {
46- return g_strdup("");
47- }
48-
49- GArray * outarray = g_array_new(TRUE, FALSE, sizeof(gchar *));
50- g_array_set_clear_func(outarray, free_string);
51-
52- int i;
53- for (i = 0; uri_split[i] != NULL; i++) {
54- gchar * path = uri2file(uri_split[i]);
55- g_array_append_val(outarray, path);
56- }
57-
58- gchar * filelist = g_strjoinv(" ", (gchar **)outarray->data);
59- g_array_free(outarray, TRUE);
60-
61- g_strfreev(uri_split);
62-
63- return filelist;
64-}
65-
66-/* Make sure we have the single URI variable */
67-static inline void
68-ensure_singleuri (gchar ** single_uri, const gchar * uri_list)
69-{
70- if (uri_list == NULL) {
71- return;
72- }
73-
74- if (*single_uri != NULL) {
75- return;
76- }
77-
78- gchar ** uri_split = NULL;
79- if (!g_shell_parse_argv(uri_list, NULL, &uri_split, NULL)) {
80- return;
81- }
82-
83- if (uri_split[0] != NULL) {
84- *single_uri = g_strdup(uri_split[0]);
85- }
86-
87- g_strfreev(uri_split);
88-
89- return;
90-}
91-
92-/* Make sure we have a single file variable */
93-static inline void
94-ensure_singlefile (gchar ** single_file, const gchar * uri_list)
95-{
96- if (uri_list == NULL) {
97- return;
98- }
99-
100- if (*single_file != NULL) {
101- return;
102- }
103-
104- gchar ** uri_split = NULL;
105- if (!g_shell_parse_argv(uri_list, NULL, &uri_split, NULL)) {
106- return;
107- }
108-
109- gchar * first_file = NULL;
110- if (uri_split[0] != NULL) {
111- first_file = uri2file(uri_split[0]);
112- }
113-
114- g_strfreev(uri_split);
115-
116- if (first_file != NULL) {
117- *single_file = first_file;
118- }
119-
120- return;
121-}
122-
123-/* Parse a desktop exec line and return the next string */
124-static gchar *
125-desktop_exec_segment_parse (const gchar * execline, const gchar * uri_list)
126-{
127- gchar ** execsplit = g_strsplit(execline, "%", 0);
128+desktop_exec_segment_parse (GArray * finalarray, const gchar * execsegment, gchar ** uri_list)
129+{
130+ /* No NULL strings */
131+ if (execsegment == NULL || execsegment[0] == '\0')
132+ return;
133+
134+ /* Handle %F and %U as an argument on their own as per the spec */
135+ if (g_strcmp0(execsegment, "%U") == 0) {
136+ return file_list_handling(finalarray, uri_list, g_strdup);
137+ }
138+ if (g_strcmp0(execsegment, "%F") == 0) {
139+ return file_list_handling(finalarray, uri_list, uri2file);
140+ }
141+
142+ /* Start looking at individual codes */
143+ gchar ** execsplit = g_strsplit(execsegment, "%", 0);
144
145 /* If we didn't have any codes, just exit here */
146 if (execsplit[1] == NULL) {
147 g_strfreev(execsplit);
148- return g_strdup(execline);
149- }
150-
151- if (uri_list != NULL && uri_list[0] == '\0') {
152- uri_list = NULL;
153+ gchar * dup = g_strdup(execsegment);
154+ g_array_append_val(finalarray, dup);
155+ return;
156 }
157
158 int i;
159- gchar * single_uri = NULL;
160- gchar * single_file = NULL;
161- gchar * file_list = NULL;
162+
163 gboolean previous_percent = FALSE;
164 GArray * outarray = g_array_new(TRUE, FALSE, sizeof(const gchar *));
165 g_array_append_val(outarray, execsplit[0]);
166+ gchar * single_file = NULL;
167
168 /* The variables allowed in an exec line from the Freedesktop.org Desktop
169 File specification: http://standards.freedesktop.org/desktop-entry-spec/desktop-entry-spec-latest.html#exec-variables */
170@@ -418,22 +358,16 @@
171 g_array_append_val(outarray, skipchar);
172 break;
173 case 'f':
174- ensure_singlefile(&single_file, uri_list);
175-
176- if (single_file != NULL) {
177+ if (uri_list != NULL && uri_list[0] != NULL) {
178+ if (single_file == NULL)
179+ single_file = uri2file(uri_list[0]);
180 g_array_append_val(outarray, single_file);
181 }
182
183 g_array_append_val(outarray, skipchar);
184 break;
185 case 'F':
186- if (uri_list != NULL) {
187- if (file_list == NULL) {
188- file_list = build_file_list(uri_list);
189- }
190- g_array_append_val(outarray, file_list);
191- }
192-
193+ g_warning("Exec line segment has a '%%F' that isn't its own argument '%s', ignoring.", execsegment);
194 g_array_append_val(outarray, skipchar);
195 break;
196 case 'i':
197@@ -443,16 +377,12 @@
198 g_array_append_val(outarray, skipchar);
199 break;
200 case 'U':
201- if (uri_list != NULL) {
202- g_array_append_val(outarray, uri_list);
203- }
204+ g_warning("Exec line segment has a '%%U' that isn't its own argument '%s', ignoring.", execsegment);
205 g_array_append_val(outarray, skipchar);
206 break;
207 case 'u':
208- ensure_singleuri(&single_uri, uri_list);
209-
210- if (single_uri != NULL) {
211- g_array_append_val(outarray, single_uri);
212+ if (uri_list != NULL && uri_list[0] != NULL) {
213+ g_array_append_val(outarray, uri_list[0]);
214 }
215
216 g_array_append_val(outarray, skipchar);
217@@ -467,12 +397,14 @@
218 gchar * output = g_strjoinv(NULL, (gchar **)outarray->data);
219 g_array_free(outarray, TRUE);
220
221- g_free(single_uri);
222+ if (output != NULL && output[0] != '\0') {
223+ g_array_append_val(finalarray, output);
224+ } else {
225+ g_free(output);
226+ }
227+
228 g_free(single_file);
229- g_free(file_list);
230 g_strfreev(execsplit);
231-
232- return output;
233 }
234
235 /* Take a full exec line, split it out, parse the segments and return
236@@ -482,6 +414,7 @@
237 {
238 GError * error = NULL;
239 gchar ** splitexec = NULL;
240+ gchar ** splituris = NULL;
241 gint execitems = 0;
242
243 /* This returns from desktop file style quoting to straight strings with
244@@ -496,14 +429,29 @@
245 return NULL;
246 }
247
248+ if (urilist != NULL && urilist[0] != '\0') {
249+ g_shell_parse_argv(urilist, NULL, &splituris, &error);
250+
251+ if (error != NULL) {
252+ g_warning("Unable to parse URIs '%s': %s", urilist, error->message);
253+ g_error_free(error);
254+ /* Continuing without URIs */
255+ splituris = NULL;
256+ }
257+ }
258+
259+
260 GArray * newargv = g_array_new(TRUE, FALSE, sizeof(gchar *));
261 int i;
262 for (i = 0; i < execitems; i++) {
263- gchar * execinserted = desktop_exec_segment_parse(splitexec[i], urilist);
264- g_array_append_val(newargv, execinserted);
265+ desktop_exec_segment_parse(newargv, splitexec[i], splituris);
266 }
267 g_strfreev(splitexec);
268
269+ if (splituris != NULL) {
270+ g_strfreev(splituris);
271+ }
272+
273 /* Each string here should be its own param */
274
275 return newargv;
276
277=== modified file 'tests/helper-test.cc'
278--- tests/helper-test.cc 2013-11-18 21:20:41 +0000
279+++ tests/helper-test.cc 2013-11-22 16:42:53 +0000
280@@ -84,9 +84,8 @@
281
282 /* Little u with a NULL string */
283 output = desktop_exec_parse("foo %u", "");
284- ASSERT_EQ(output->len, 2);
285+ ASSERT_EQ(output->len, 1);
286 ASSERT_STREQ(g_array_index(output, gchar *, 0), "foo");
287- ASSERT_STREQ(g_array_index(output, gchar *, 1), "");
288 g_array_free(output, TRUE);
289
290 /* Big %U with a single URL */
291@@ -121,9 +120,8 @@
292
293 /* URL is a quote, make sure we handle the error */
294 output = desktop_exec_parse("foo %u", "\"");
295- ASSERT_EQ(output->len, 2);
296+ ASSERT_EQ(output->len, 1);
297 ASSERT_STREQ(g_array_index(output, gchar *, 0), "foo");
298- ASSERT_STREQ(g_array_index(output, gchar *, 1), "");
299 g_array_free(output, TRUE);
300
301 /* Lots of quotes, escaped and not */
302@@ -141,16 +139,23 @@
303
304 /* Big U with two URLs */
305 output = desktop_exec_parse("foo %U", "http://ubuntu.com http://slashdot.org");
306- ASSERT_EQ(output->len, 2);
307+ ASSERT_EQ(output->len, 3);
308 ASSERT_STREQ(g_array_index(output, gchar *, 0), "foo");
309- ASSERT_STREQ(g_array_index(output, gchar *, 1), "http://ubuntu.com http://slashdot.org");
310+ ASSERT_STREQ(g_array_index(output, gchar *, 1), "http://ubuntu.com");
311+ ASSERT_STREQ(g_array_index(output, gchar *, 2), "http://slashdot.org");
312 g_array_free(output, TRUE);
313
314 /* Big U with no URLs */
315 output = desktop_exec_parse("foo %U", NULL);
316+ ASSERT_EQ(output->len, 1);
317+ ASSERT_STREQ(g_array_index(output, gchar *, 0), "foo");
318+ g_array_free(output, TRUE);
319+
320+ /* Big U with quoted URL */
321+ output = desktop_exec_parse("foo %U", "'http://ubuntu.com'");
322 ASSERT_EQ(output->len, 2);
323 ASSERT_STREQ(g_array_index(output, gchar *, 0), "foo");
324- ASSERT_STREQ(g_array_index(output, gchar *, 1), "");
325+ ASSERT_STREQ(g_array_index(output, gchar *, 1), "http://ubuntu.com");
326 g_array_free(output, TRUE);
327
328 /* Big U with URLs that have spaces */
329@@ -169,16 +174,14 @@
330
331 /* A %f with a NULL string */
332 output = desktop_exec_parse("foo %f", "");
333- ASSERT_EQ(output->len, 2);
334+ ASSERT_EQ(output->len, 1);
335 ASSERT_STREQ(g_array_index(output, gchar *, 0), "foo");
336- ASSERT_STREQ(g_array_index(output, gchar *, 1), "");
337 g_array_free(output, TRUE);
338
339 /* %f with a URL that isn't a file */
340 output = desktop_exec_parse("foo %f", "torrent://moviephone.com/hot-new-movie");
341- ASSERT_EQ(output->len, 2);
342+ ASSERT_EQ(output->len, 1);
343 ASSERT_STREQ(g_array_index(output, gchar *, 0), "foo");
344- ASSERT_STREQ(g_array_index(output, gchar *, 1), "");
345 g_array_free(output, TRUE);
346
347 /* Lots of %f combinations */
348@@ -200,16 +203,16 @@
349
350 /* Big F with two files */
351 output = desktop_exec_parse("foo %F", "file:///proc/version file:///proc/uptime");
352- ASSERT_EQ(output->len, 2);
353+ ASSERT_EQ(output->len, 3);
354 ASSERT_STREQ(g_array_index(output, gchar *, 0), "foo");
355- ASSERT_STREQ(g_array_index(output, gchar *, 1), "/proc/version /proc/uptime");
356+ ASSERT_STREQ(g_array_index(output, gchar *, 1), "/proc/version");
357+ ASSERT_STREQ(g_array_index(output, gchar *, 2), "/proc/uptime");
358 g_array_free(output, TRUE);
359
360 /* Big F with no files */
361 output = desktop_exec_parse("foo %F", NULL);
362- ASSERT_EQ(output->len, 2);
363+ ASSERT_EQ(output->len, 1);
364 ASSERT_STREQ(g_array_index(output, gchar *, 0), "foo");
365- ASSERT_STREQ(g_array_index(output, gchar *, 1), "");
366 g_array_free(output, TRUE);
367
368 /* Groups of percents */
369@@ -223,17 +226,8 @@
370
371 /* All the % sequences we don't support */
372 output = desktop_exec_parse("foo %d %D %n %N %v %m %i %c %k", "file:///proc/version");
373- ASSERT_EQ(output->len, 10);
374+ ASSERT_EQ(output->len, 1);
375 ASSERT_STREQ(g_array_index(output, gchar *, 0), "foo");
376- ASSERT_STREQ(g_array_index(output, gchar *, 1), "");
377- ASSERT_STREQ(g_array_index(output, gchar *, 2), "");
378- ASSERT_STREQ(g_array_index(output, gchar *, 3), "");
379- ASSERT_STREQ(g_array_index(output, gchar *, 4), "");
380- ASSERT_STREQ(g_array_index(output, gchar *, 5), "");
381- ASSERT_STREQ(g_array_index(output, gchar *, 6), "");
382- ASSERT_STREQ(g_array_index(output, gchar *, 7), "");
383- ASSERT_STREQ(g_array_index(output, gchar *, 8), "");
384- ASSERT_STREQ(g_array_index(output, gchar *, 9), "");
385 g_array_free(output, TRUE);
386
387 return;

Subscribers

People subscribed via source and target branches