all repos — openbox @ a6141fe7a4895593d412a18bd24d1859d2d69c34

openbox fork - make it a bit more like ryudo

improved .desktop parsing.

properly (and quickly) check for existence of required keys
figure out what an app can open from its exec key
validate the %fields in an app's exec key
Dana Jansens danakj@orodu.net
commit

a6141fe7a4895593d412a18bd24d1859d2d69c34

parent

d9d65b73853d485852f6d6bf6808af0ebb6a90f5

3 files changed, 158 insertions(+), 53 deletions(-)

jump to
M obt/ddparse.cobt/ddparse.c

@@ -35,9 +35,19 @@ typedef gboolean (*ObtDDParseValueFunc)(gchar *key, const gchar *val,

ObtDDParse *parse, gboolean *error); +enum { + DE_TYPE = 1 << 0, + DE_TYPE_APPLICATION = 1 << 1, + DE_TYPE_LINK = 1 << 2, + DE_NAME = 1 << 3, + DE_EXEC = 1 << 4, + DE_URL = 1 << 5 +}; + struct _ObtDDParse { gchar *filename; gulong lineno; + gulong flags; ObtDDParseGroup *group; /* the key is a group name, the value is a ObtDDParseGroup */ GHashTable *group_hash;

@@ -69,6 +79,7 @@

static void parse_value_free(ObtDDParseValue *v) { switch (v->type) { + case OBT_DDPARSE_EXEC: case OBT_DDPARSE_STRING: case OBT_DDPARSE_LOCALESTRING: g_free(v->value.string); break;

@@ -79,7 +90,7 @@ v->value.strings.n = 0;

break; case OBT_DDPARSE_BOOLEAN: case OBT_DDPARSE_NUMERIC: - case OBT_DDPARSE_ENUM_APPLICATION: + case OBT_DDPARSE_ENUM_TYPE: case OBT_DDPARSE_ENVIRONMENTS: break; default:

@@ -146,7 +157,12 @@ out = g_new(char, bytes + 1);

i = in; o = out; backslash = FALSE; while (i < end) { - const gchar *next = locale ? g_utf8_find_next_char(i, end) : i+1; + const gchar *next; + + /* find the next character in the string */ + if (!locale) next = i+1; + else if (!(next = g_utf8_find_next_char(i, end))) next = end; + if (backslash) { switch(*i) { case 's': *o++ = ' '; break;

@@ -181,7 +197,7 @@ }

i = next; } *o = '\0'; - return o; + return out; } static guint parse_value_environments(const gchar *in,

@@ -189,7 +205,6 @@ const ObtDDParse *const parse,

gboolean *error) { const gchar *s; - int i; guint mask = 0; s = in;

@@ -413,9 +428,10 @@ ((guchar)buf[i] >= 'a' && (guchar)buf[i] <= 'z') ||

((guchar)buf[i] >= '0' && (guchar)buf[i] <= '9') || ((guchar)buf[i] == '-'))) { /* not part of the key */ - keyend = i; break; } + keyend = i; + if (keyend < 1) { parse_error("Empty key", parse, error); return;

@@ -505,7 +521,7 @@ }

break; case 'E': /* Exec */ if (strcmp(key+1, "xec")) return FALSE; - v.type = OBT_DDPARSE_STRING; break; + v.type = OBT_DDPARSE_EXEC; parse->flags |= DE_EXEC; break; case 'G': /* GenericName */ if (strcmp(key+1, "enericName")) return FALSE; v.type = OBT_DDPARSE_LOCALESTRING; break;

@@ -522,7 +538,7 @@ case 'N':

switch (key[1]) { case 'a': /* Name */ if (strcmp(key+2, "me")) return FALSE; - v.type = OBT_DDPARSE_LOCALESTRING; break; + v.type = OBT_DDPARSE_LOCALESTRING; parse->flags |= DE_NAME; break; case 'o': switch (key[2]) { case 'D': /* NoDisplay */

@@ -568,14 +584,14 @@ if (strcmp(key+2, "yExec")) return FALSE;

v.type = OBT_DDPARSE_STRING; break; case 'y': /* Type */ if (strcmp(key+2, "pe")) return FALSE; - v.type = OBT_DDPARSE_STRING; break; + v.type = OBT_DDPARSE_ENUM_TYPE; parse->flags |= DE_TYPE; break; default: return FALSE; } break; case 'U': /* URL */ if (strcmp(key+1, "RL")) return FALSE; - v.type = OBT_DDPARSE_STRING; break; + v.type = OBT_DDPARSE_STRING; parse->flags |= DE_URL; break; case 'V': /* MimeType */ if (strcmp(key+1, "ersion")) return FALSE; v.type = OBT_DDPARSE_STRING; break;

@@ -585,6 +601,57 @@ }

/* parse the value */ switch (v.type) { + case OBT_DDPARSE_EXEC: { + gchar *c, *m; + gboolean percent; + gboolean found; + + v.value.string = parse_value_string(val, FALSE, NULL, parse, error); + g_assert(v.value.string); + + /* an exec string can only contain one of the file/url-opening %'s */ + percent = found = FALSE; + for (c = v.value.string; *c; ++c) { + if (*c == '%') percent = !percent; + if (percent) { + switch (*c) { + case 'f': + case 'F': + case 'u': + case 'U': + if (found) { + m = g_strdup_printf("Malformed Exec key, " + "extraneous %%%c", *c); + parse_error(m, parse, error); + g_free(m); + } + found = TRUE; + break; + case 'd': + case 'D': + case 'n': + case 'N': + case 'v': + case 'm': + m = g_strdup_printf("Malformed Exec key, " + "uses deprecated %%%c", *c); + parse_error(m, parse, NULL); /* just a warning */ + g_free(m); + break; + case 'i': + case 'c': + case 'k': + break; + default: + m = g_strdup_printf("Malformed Exec key, " + "uses unknown %%%c", *c); + parse_error(m, parse, NULL); /* just a warning */ + g_free(m); + } + } + } + break; + } case OBT_DDPARSE_STRING: v.value.string = parse_value_string(val, FALSE, NULL, parse, error); g_assert(v.value.string);

@@ -611,11 +678,15 @@ break;

case OBT_DDPARSE_NUMERIC: v.value.numeric = parse_value_numeric(val, parse, error); break; - case OBT_DDPARSE_ENUM_APPLICATION: - if (val[0] == 'A' && strcmp(val+1, "pplication") == 0) + case OBT_DDPARSE_ENUM_TYPE: + if (val[0] == 'A' && strcmp(val+1, "pplication") == 0) { v.value.enumerable = OBT_LINK_TYPE_APPLICATION; - else if (val[0] == 'L' && strcmp(val+1, "ink") == 0) + parse->flags |= DE_TYPE_APPLICATION; + } + else if (val[0] == 'L' && strcmp(val+1, "ink") == 0) { v.value.enumerable = OBT_LINK_TYPE_URL; + parse->flags |= DE_TYPE_LINK; + } else if (val[0] == 'D' && strcmp(val+1, "irectory") == 0) v.value.enumerable = OBT_LINK_TYPE_DIRECTORY; else {

@@ -663,17 +734,40 @@ gchar *path = g_strdup_printf("%s/%s", (char*)it->data, name);

if ((f = fopen(path, "r"))) { parse.filename = path; parse.lineno = 1; - success = parse_file(f, &parse); + parse.flags = 0; + if ((success = parse_file(f, &parse))) { + /* check that required keys exist */ + + if (!(parse.flags & DE_TYPE)) { + g_warning("Missing Type key in %s", path); + success = FALSE; + } + if (!(parse.flags & DE_NAME)) { + g_warning("Missing Name key in %s", path); + success = FALSE; + } + if (parse.flags & DE_TYPE_APPLICATION && + !(parse.flags & DE_EXEC)) + { + g_warning("Missing Exec key for Application in %s", + path); + success = FALSE; + } + else if (parse.flags & DE_TYPE_LINK && !(parse.flags & DE_URL)) + { + g_warning("Missing URL key for Link in %s", path); + success = FALSE; + } + } fclose(f); } g_free(path); } if (!success) { g_hash_table_destroy(parse.group_hash); - return NULL; + parse.group_hash = NULL; } - else - return parse.group_hash; + return parse.group_hash; } GHashTable* obt_ddparse_group_keys(ObtDDParseGroup *g)
M obt/ddparse.hobt/ddparse.h

@@ -21,13 +21,14 @@

typedef struct _ObtDDParseGroup ObtDDParseGroup; typedef enum { + OBT_DDPARSE_EXEC, OBT_DDPARSE_STRING, OBT_DDPARSE_LOCALESTRING, OBT_DDPARSE_STRINGS, OBT_DDPARSE_LOCALESTRINGS, OBT_DDPARSE_BOOLEAN, OBT_DDPARSE_NUMERIC, - OBT_DDPARSE_ENUM_APPLICATION, + OBT_DDPARSE_ENUM_TYPE, OBT_DDPARSE_ENVIRONMENTS, OBT_DDPARSE_NUM_VALUE_TYPES } ObtDDParseValueType;
M obt/link.cobt/link.c

@@ -66,46 +66,26 @@ {

ObtLink *link; GHashTable *groups, *keys; ObtDDParseGroup *g; - ObtDDParseValue *v, *type, *name, *target; + ObtDDParseValue *v; + /* parse the file, and get a hash table of the groups */ groups = obt_ddparse_file(ddname, paths); - if (!groups) return NULL; + if (!groups) return NULL; /* parsing failed */ + /* grab the Desktop Entry group */ g = g_hash_table_lookup(groups, "Desktop Entry"); - if (!g) { - g_hash_table_destroy(groups); - return NULL; - } - + g_assert(g != NULL); + /* grab the keys that appeared in the Desktop Entry group */ keys = obt_ddparse_group_keys(g); - /* check that required keys exist */ - - if (!(type = g_hash_table_lookup(keys, "Type"))) - { g_hash_table_destroy(groups); return NULL; } - if (!(name = g_hash_table_lookup(keys, "Name"))) - { g_hash_table_destroy(groups); return NULL; } - - if (type->value.enumerable == OBT_LINK_TYPE_APPLICATION) { - if (!(target = g_hash_table_lookup(keys, "Exec"))) - { g_hash_table_destroy(groups); return NULL; } - } - else if (type->value.enumerable == OBT_LINK_TYPE_URL) { - if (!(target = g_hash_table_lookup(keys, "URL"))) - { g_hash_table_destroy(groups); return NULL; } - } - else - target = NULL; - - /* parse all the optional keys and build ObtLink (steal the strings) */ + /* build the ObtLink (we steal all strings from the parser) */ link = g_slice_new0(ObtLink); link->ref = 1; - link->type = type->value.enumerable; - if (link->type == OBT_LINK_TYPE_APPLICATION) - link->d.app.exec = target->value.string, target->value.string = NULL; - else if (link->type == OBT_LINK_TYPE_URL) - link->d.url.addr = target->value.string, target->value.string = NULL; link->display = TRUE; + v = g_hash_table_lookup(keys, "Type"); + g_assert(v); + link->type = v->value.enumerable; + if ((v = g_hash_table_lookup(keys, "Hidden"))) link->deleted = v->value.boolean;

@@ -131,7 +111,31 @@ link->env_restricted = v->value.environments;

else link->env_restricted = 0; + /* type-specific keys */ + if (link->type == OBT_LINK_TYPE_APPLICATION) { + gchar *c; + gboolean percent; + + v = g_hash_table_lookup(keys, "Exec"); + g_assert(v); + link->d.app.exec = v->value.string; + v->value.string = NULL; + + /* parse link->d.app.exec to determine link->d.app.open */ + percent = FALSE; + for (c = link->d.app.exec; *c; ++c) { + if (*c == '%') percent = !percent; + if (percent) { + switch (*c) { + case 'f': link->d.app.open = OBT_LINK_APP_SINGLE_LOCAL; break; + case 'F': link->d.app.open = OBT_LINK_APP_MULTI_LOCAL; break; + case 'u': link->d.app.open = OBT_LINK_APP_SINGLE_URL; break; + case 'U': link->d.app.open = OBT_LINK_APP_MULTI_URL; break; + } + } + } + if ((v = g_hash_table_lookup(keys, "TryExec"))) { /* XXX spawn a thread to check TryExec? */ link->display = link->display &&

@@ -151,16 +155,22 @@ if ((v = g_hash_table_lookup(keys, "StartupNotify")))

link->d.app.startup = v->value.boolean ? OBT_LINK_APP_STARTUP_PROTOCOL_SUPPORT : OBT_LINK_APP_STARTUP_NO_SUPPORT; - else + else { link->d.app.startup = OBT_LINK_APP_STARTUP_LEGACY_SUPPORT; - - /* XXX parse link->d.app.exec to determine link->d.app.open */ + if ((v = g_hash_table_lookup(keys, "StartupWMClass"))) { + /* steal the string */ + link->d.app.startup_wmclass = v->value.string; + v->value.string = NULL; + } + } /* XXX there's more app specific stuff */ } - else if (link->type == OBT_LINK_TYPE_URL) { - /* XXX there's URL specific stuff */ + v = g_hash_table_lookup(keys, "URL"); + g_assert(v); + link->d.url.addr = v->value.string; + v->value.string = NULL; } return link;