Bug 28826

-        // as an extension to the Desktop Entry spec, we allow "_", "." and "@"
+        // as an extension to the Desktop Entry spec, we allow " ", "_", "." and "@"
         // as valid key characters - "_" and "." are needed for keys that are
         // D-Bus property names, and GKeyFile and KConfigIniBackend also accept
         // all three of those characters.

There are now four extra characters, not three. Do GKF and KCIB both also accept space?

(In reply to comment #0)
> Also if a string list does not terminate with ; KeyFile::valueAsStringList
> misses the last string.

So this is being permissive when parsing malformed keyfiles?

There should be a regression test for a key containing a space.

There should be regression tests for string lists that do and don't end with ";". The protocol "somewhat-pathological" in tests/telepathy/managers/test-manager-file.manager is an example of what test data for this sort of thing ought to look like: please parse all the default values.

> -        // as an extension to the Desktop Entry spec, we allow "_", "." and "@"
> +        // as an extension to the Desktop Entry spec, we allow " ", "_", "." and "@"
>          // as valid key characters - "_" and "." are needed for keys that are
>          // D-Bus property names, and GKeyFile and KConfigIniBackend also accept
>          // all three of those characters.

All four of those characters? :-P

Fact-check: does KConfigIniBackend actually accept those? I hope so...

FYI, GKeyFile rejects spaces at the beginning or end of a key, and has special handling for square brackets (they must be paired and can only contain alphanumerics or "-_.@", which are enough for all locales).

We should watch out for this in telepathy-spec in future...

(In reply to comment #2)
> There should be a regression test for a key containing a space.
> 
> There should be regression tests for string lists that do and don't end with
> ";". The protocol "somewhat-pathological" in
> tests/telepathy/managers/test-manager-file.manager is an example of what test
> data for this sort of thing ought to look like: please parse all the default
> values.
Done

> > -        // as an extension to the Desktop Entry spec, we allow "_", "." and "@"
> > +        // as an extension to the Desktop Entry spec, we allow " ", "_", "." and "@"
> >          // as valid key characters - "_" and "." are needed for keys that are
> >          // D-Bus property names, and GKeyFile and KConfigIniBackend also accept
> >          // all three of those characters.
> 
> All four of those characters? :-P
Done

> 
> Fact-check: does KConfigIniBackend actually accept those? I hope so...
Just checked and KCIB does accept _everything_ as key values. It finds the first = and everything before it is the key. The key can have [] which are treated specially.

Note that we do not support [] yet. We can always add support later when/if needed.

> FYI, GKeyFile rejects spaces at the beginning or end of a key, and has special
> handling for square brackets (they must be paired and can only contain
> alphanumerics or "-_.@", which are enough for all locales).
We also ignore spaces in the begin/end of a key (We trim the string). We need to support spaces (not in the begin/end), cause RequestableChannelClasses need it. Unless we change RequestableChannelClasses definition in .manager file, we need it.

> We should watch out for this in telepathy-spec in future...
Indeed

Merged upstream. It will be in next release 0.3.6.