[Bug 30296] Add Conn.I.Addressing support to Gabble

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=30296

--- Comment #18 from Simon McVittie <simon.mcvittie at collabora.co.uk> 2011-11-17 03:49:05 PST ---
(In reply to comment #17)
> > then its correct URI form is "xmpp:wtf%63 at example.com"
> "xmpp:wtf%3F at example.com"

To clarify: you're right, I was misreading ascii(7). (63 = 0x3F)

> I am escaping the jid components separately (escape(domain), escape(...)), as
> escaping the whole jid leads to "@" being escaped, but I am not sure if the
> domain and resource should be escaped also.

Yes they should. I don't think it can actually matter for a correct domain, but
the resource can contain all sorts of strange things (spaces!), and what you're
doing here is the maximally-correct thing.

> + <p>A mapping from requested vCard addresses and the corresponding

from requested vCard addresses *to* the corresponding

> + g_hash_table_insert (requested, (gpointer) g_strdup (*uri),
> GUINT_TO_POINTER (h));

The (gpointer) cast is unnecessary now - we were using it as the C equivalent
of const_cast<>, and now that you g_strdup the keys, they aren't const anyway.

> +gabble_uri_to_jid (const gchar *uri,

This can't tell the difference between component boundaries and escaped
component boundaries in the URI: if you feed it certain invalid URIs like
xmpp:invalid%40domain.example.com, it will re-interpret the escaped @ as an
unescaped @, and consider it to be invalid at example.com, rather than noticing
that "invalid at domain" isn't a valid hostname.

I'm not sure that this is really a practical problem, though. Perhaps clone it
as a bug and get on with your life?

To fix this we'd need to have a function like:

gboolean gabble_parse_xmpp_uri (const gchar *uri,
                                gchar **node_or_null,
                                gchar **domain,
                                gchar **resource_or_null)

which split at @ and / *first*, then unescaped each part individually, then
finally checked each part for validity. Given that (iirc) Wocky doesn't yet do
proper nodeprep/resourceprep to validate/normalize nodes and resources in JIDs
according to the RFCs, I think this is a job for another bug.

> +gabble_jid_to_uri (const gchar *scheme,
[...]
> + uri = g_strdup_printf ("%s:%s", scheme, jid);
> +
> + normalized_uri = gabble_normalize_uri (uri, error);

This doesn't look right: you're relying on gabble_normalize_uri accepting
insufficiently-escaped JIDs. Which it does, at the moment, because it starts
with a call to gabble_uri_to_jid, which is buggy as described above... but
still.

I think you should move most of the body of gabble_normalize_uri into this
function, so that this function starts at the gabble_decode_jid call; then
gabble_normalize_uri can just be

    gabble_jid_to_uri (gabble_uri_to_jid (uri))

(except with memory management and error handling).

I'd like to see some non-URI-allowed characters (using "?" again would do
nicely) in the regression test's resource, to check that that works.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA Contact for the bug.