bosh.c: remove HTTP Pipelining, fix #17025

Pidgin attempts to use HTTP Pipelining to avoid creating multiple connections.
Unfortunately, it may work for some servers (if you are lucky), but this practice
causes incompatibility for many other servers, especially if reverse proxies or
intermediate proxies are presented (which is a common use case of BOSH).

Also, HTTP Pipelining for BOSH is wrong in the first place because it is prohibited
by RFC-2616 and XEP-0124:

RFC-2616, Section 8: non-idempotent requests (like POST) should not be pipelined.
XEP-0124, 18.1: clients and connection managers SHOULD NOT use HTTP Pipelining.

This commit removes HTTP Pipelining from the 2.x.y branch, in order to fix bugs
caused by HTTP Pipelining, such as https://developer.pidgin.im/ticket/17025.

In fact, HTTP Pipelining in Pidgin has been disabled as early as 2009, e.g. 208dfa375b79,
but never went through the stable release.

1 files changed, 0 insertions, 35 deletions

diff --git a/libpurple/protocols/jabber/bosh.c b/libpurple/protocols/jabber/bosh.c
index af178c64bc..d80ecd6ac1 100644
--- a/libpurple/protocols/jabber/bosh.c
+++ b/libpurple/protocols/jabber/bosh.c
@@ -68,7 +68,6 @@ struct _PurpleBOSHConnection {
 	char *path;
 	guint16 port;
 
-	gboolean pipelining;
 	gboolean ssl;
 
 	enum {
@@ -212,7 +211,6 @@ jabber_bosh_connection_init(JabberStream *js, const char *url)
 	conn->port = port;
 	conn->path = g_strdup_printf("/%s", path);
 	g_free(path);
-	conn->pipelining = TRUE;
 
 	if (purple_ip_address_is_valid(host))
 		js->serverFQDN = g_strdup(js->user->domain);
@@ -285,12 +283,6 @@ find_available_http_connection(PurpleBOSHConnection *conn)
 	if (purple_debug_is_verbose())
 		debug_dump_http_connections(conn);
 
-	/* Easy solution: Does everyone involved support pipelining? Hooray! Just use
-	 * one TCP connection! */
-	if (conn->pipelining)
-		return conn->connections[0]->state == HTTP_CONN_CONNECTED ?
-				conn->connections[0] : NULL;
-
 	/* First loop, look for a connection that's ready */
 	for (i = 0; i < NUM_HTTP_CONNECTIONS; ++i) {
 		if (conn->connections[i] &&
@@ -465,27 +457,6 @@ jabber_bosh_connection_send_keepalive(PurpleBOSHConnection *bosh)
 	send_timer_cb(bosh);
 }
 
-static void
-jabber_bosh_disable_pipelining(PurpleBOSHConnection *bosh)
-{
-	/* Do nothing if it's already disabled */
-	if (!bosh->pipelining)
-		return;
-
-	purple_debug_info("jabber", "BOSH: Disabling pipelining on conn %p\n",
-	                            bosh);
-	bosh->pipelining = FALSE;
-	if (bosh->connections[1] == NULL) {
-		bosh->connections[1] = jabber_bosh_http_connection_init(bosh);
-		http_connection_connect(bosh->connections[1]);
-	} else {
-		/* Shouldn't happen; this should be the only place pipelining
-		 * is turned off.
-		 */
-		g_warn_if_reached();
-	}
-}
-
 static void jabber_bosh_connection_received(PurpleBOSHConnection *conn, xmlnode *node) {
 	xmlnode *child;
 	JabberStream *js = conn->js;
@@ -726,11 +697,6 @@ static void http_connection_disconnected(PurpleHTTPConnection *conn)
 		conn->requests = 0;
 	}
 
-	if (conn->bosh->pipelining) {
-		/* Hmmmm, fall back to multiple connections */
-		jabber_bosh_disable_pipelining(conn->bosh);
-	}
-
 	if (!had_requests)
 		/* If the server disconnected us without any requests, let's
 		 * just wait until we have something to send before we reconnect
@@ -807,7 +773,6 @@ jabber_bosh_http_connection_process(PurpleHTTPConnection *conn)
 
 			if (!g_ascii_strncasecmp(tmp, "close", strlen("close"))) {
 				conn->close = TRUE;
-				jabber_bosh_disable_pipelining(conn->bosh);
 			}
 		}