[Bug 23640] [regression] Re-implement proxy support

[bugzilla-daemon at freedesktop.org]

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

--- Comment #4 from Simon McVittie <simon.mcvittie at collabora.co.uk> 2010-09-06 10:16:31 PDT ---
I'm sad that we're parsing strings from a list, when we have a perfectly good
structured type system, but I can see why you did this (we don't have a sane
serialization for anything else in a keyfile) and I can't think of anything
better we could do.

> +  guint fallback_server_idx;

..._index - please avoid uncommon abbreviations.

It would be nice to have a concrete example of what we would use for Google
Talk, either in the code or in the commit message. I infer that it would be
something like this:

    ["talk.l.google.com",
    "talk.l.google.com,443,1"]

The commit message says the syntax has colons and the implementation says it
has commas: please fix one or the other.

I assume that the reason for using commas was that colons conflict with IPv6
literals' use of colons, but using commas is weird and unconventional too.
Possibilities:

* use commas anyway
* use colons, support URI-style IPv6 literals like [::1]:5222 (hard,
pointless?)
* use colons, don't support IPv6 literals, assume that anyone with working IPv6
will also have working DNS

Given its purpose, it's not at all clear to me that we'd ever want IP literals
in the fallback server list at all, let alone IPv6 literals.

> +        old_ssl = strcmp (split[2],"1") == 0;

The part after " = " should have parentheses for clarity, or (better) be
implemented in terms of tp_strdiff().

I don't much like this syntax for old-SSL anyway, though; if nothing else,
",oldssl" is more mnemonic than ",1".

Are we ever likely to want to fall back to old-SSL on a port that is neither
5223 nor 443, or normal XMPP on port 5223 or 443? If not, we could perhaps
decide whether to use old-SSL based on the port number?

(That question basically means: are we ever likely to use this for a service
that isn't Google Talk, and if so, are we ever likely to use it for a service
that is very strangely set up?)

> +next_fallback_server (GabbleConnection *self, const GError *error)

Style: new line for the second argument please

> + * FALSE is they all have been tried.

Typo: "FALSE if they"

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