[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

[Libevent-users] [PATCH] client HTTPS with evhttp



I've written up a patch that allows evhttp to work for HTTPS where a second (or further) request is sent on a evhttp_connection that has previously timed out and closed its underlying TCP connection. Fortunately, it was a fairly simple addition. Here's a rundown of what it does:

1) adds the evhttp_connection_base_bufferevent_factory_new() API call that takes a callback function to create new bufferevents.

2) adds the callback (and the user-supplied void * argument to the callback) to the evhttp_connection struct

3) if the callback is set, calls it to generate a new bufferevent for the connection in evhttp_connection_connect(). I added a flag that tracks whether the bufferevent has been used yet so we can avoid regenerating bufferevents spuriously.

4) resets the bufferevent file descriptor to -1 before freeing it to ensure the bufferevent won't prematurely close our file descriptors if it happens to have it's FREE_ON_CLOSE flag set. This was a problem since we re-use file descriptor numbers, and a just-allocated fd=7 could get incorrectly freed if the old, timed-out fd was also 7 and we free the old bufferevent. The patch solves this problem for the case where a bufferevent callback is not used, too.

Thanks!
Myk
diff --git a/include/event2/http.h b/include/event2/http.h
index 5c9f0e5..5372208 100644
--- a/include/event2/http.h
+++ b/include/event2/http.h
@@ -430,15 +430,17 @@ enum evhttp_cmd_type {
 enum evhttp_request_kind { EVHTTP_REQUEST, EVHTTP_RESPONSE };
 
 /**
- * Create and return a connection object that can be used to for making HTTP
+ * Create and return a connection object that can be used for making HTTP
  * requests.  The connection object tries to resolve address and establish the
- * connection when it is given an http request object.
+ * connection when it is given an http request object.  The specified
+ * bufferevent will be reused even if the underlying persistent HTTP connection
+ * goes down and needs to be reestablished.
  *
  * @param base the event_base to use for handling the connection
  * @param dnsbase the dns_base to use for resolving host names; if not
  *     specified host name resolution will block.
  * @param bev a bufferevent to use for connecting to the server; if NULL, a
- *     socket-based bufferevent will be created.  This buffrevent will be freed
+ *     socket-based bufferevent will be created.  This bufferevent will be freed
  *     when the connection closes.  It must have no fd set on it.
  * @param address the address to which to connect
  * @param port the port to connect to
@@ -448,6 +450,38 @@ struct evhttp_connection *evhttp_connection_base_bufferevent_new(
 	struct event_base *base, struct evdns_base *dnsbase, struct bufferevent* bev, const char *address, unsigned short port);
 
 /**
+ * Creates and returns a new bufferevent object.
+ */
+typedef struct bufferevent* (*bev_factory_cb)(void *);
+
+/**
+ * Create and return a connection object that can be used for making HTTP
+ * requests.  The connection object tries to resolve address and establish the
+ * connection when it is given an http request object.  The specified factory
+ * function is called with the user-supplied argument to retrieve a new
+ * bufferevent whenever the underlying HTTP connection needs to be
+ * reestablished.  This is what you want if, for example, you have a bufferevent
+ * that needs to perform some setup for new connections, such as an SSL
+ * bufferevent.
+ *
+ * @param base the event_base to use for handling the connection
+ * @param dnsbase the dns_base to use for resolving host names; if not
+ *     specified host name resolution will block.
+ * @param cb a callback that returns a new bufferevent to use for connecting to
+ *     the server; if NULL, behavior is the same as in calling
+ *     evhttp_connection_base_bufferevent_new with a NULL bufferevent.  The
+ *     returned bufferevents will be freed as necessary.  The returned
+ *     bufferevents must have no fd set on them.
+ * @param arg the argument to supply to the callback
+ * @param address the address to which to connect
+ * @param port the port to connect to
+ * @return an evhttp_connection object that can be used for making requests
+ */
+struct evhttp_connection *evhttp_connection_base_bufferevent_factory_new(
+	struct event_base *base, struct evdns_base *dnsbase,
+	bev_factory_cb cb, void * arg, const char *address, unsigned short port);
+
+/**
  * Return the bufferevent that an evhttp_connection is using.
  */
 struct bufferevent* evhttp_connection_get_bufferevent(struct evhttp_connection *evcon);
@@ -474,9 +508,11 @@ void evhttp_request_set_chunked_cb(struct evhttp_request *,
 void evhttp_request_free(struct evhttp_request *req);
 
 /**
- * Create and return a connection object that can be used to for making HTTP
+ * Create and return a connection object that can be used for making HTTP
  * requests.  The connection object tries to resolve address and establish the
- * connection when it is given an http request object.
+ * connection when it is given an http request object.  This function is
+ * equivalent to calling evhttp_connection_base_bufferevent_new with a NULL
+ * bufferevent.
  *
  * @param base the event_base to use for handling the connection
  * @param dnsbase the dns_base to use for resolving host names; if not
diff --git a/http-internal.h b/http-internal.h
index 7fab641..12ff349 100644
--- a/http-internal.h
+++ b/http-internal.h
@@ -13,6 +13,7 @@
 #include "event2/event_struct.h"
 #include "util-internal.h"
 #include "defer-internal.h"
+#include "event2/http.h"
 
 #define HTTP_CONNECT_TIMEOUT	45
 #define HTTP_WRITE_TIMEOUT	50
@@ -66,6 +67,8 @@ struct evhttp_connection {
 
 	evutil_socket_t fd;
 	struct bufferevent *bufev;
+	bev_factory_cb bufcb;
+	void *bufcb_arg;
 
 	struct event retry_ev;		/* for retrying connects */
 
@@ -82,6 +85,7 @@ struct evhttp_connection {
 #define EVHTTP_CON_INCOMING	0x0001	/* only one request on it ever */
 #define EVHTTP_CON_OUTGOING	0x0002  /* multiple requests possible */
 #define EVHTTP_CON_CLOSEDETECT  0x0004  /* detecting if persistent close */
+#define EVHTTP_BEV_USED  0x0008  /* mark whether we need to allocate a new bufferevent */
 
 	struct timeval timeout;		/* timeout for events */
 	int retry_cnt;			/* retry count */
diff --git a/http.c b/http.c
index 1fccc2c..1177da0 100644
--- a/http.c
+++ b/http.c
@@ -1152,8 +1152,11 @@ evhttp_connection_free(struct evhttp_connection *evcon)
 		event_debug_unassign(&evcon->retry_ev);
 	}
 
-	if (evcon->bufev != NULL)
+	if (evcon->bufev != NULL) {
+		/* ensure freeing doesn't prematurely close the file descriptor */
+		bufferevent_setfd(evcon->bufev, -1);
 		bufferevent_free(evcon->bufev);
+	}
 
 	event_deferred_cb_cancel(get_deferred_queue(evcon),
 	    &evcon->read_more_deferred_cb);
@@ -1207,6 +1210,7 @@ evhttp_request_dispatch(struct evhttp_connection* evcon)
 	EVUTIL_ASSERT(evcon->state == EVCON_IDLE);
 
 	evcon->state = EVCON_WRITING;
+	evcon->flags |= EVHTTP_BEV_USED;
 
 	/* Create the header from the store arguments */
 	evhttp_make_header(evcon, req);
@@ -2212,6 +2216,30 @@ evhttp_connection_base_bufferevent_new(struct event_base *base, struct evdns_bas
 	return (NULL);
 }
 
+struct evhttp_connection *evhttp_connection_base_bufferevent_factory_new(
+	struct event_base *base, struct evdns_base *dnsbase,
+	bev_factory_cb cb, void * arg, const char *address, unsigned short port)
+{
+	struct bufferevent *bev = NULL;
+
+	if (NULL != cb) {
+		if (NULL == (bev = (*cb)(arg))) {
+			event_warn("%s: bufferevent factory callback failed", __func__);
+			return (NULL);
+		}
+	}
+
+	struct evhttp_connection *ret =
+		evhttp_connection_base_bufferevent_new(base, dnsbase, bev, address, port);
+    
+	if (NULL != ret) {
+		ret->bufcb = cb;
+		ret->bufcb_arg = arg;
+	}
+
+	return (ret);
+}
+
 struct bufferevent* evhttp_connection_get_bufferevent(struct evhttp_connection *evcon)
 {
 	return evcon->bufev;
@@ -2305,6 +2333,30 @@ evhttp_connection_connect(struct evhttp_connection *evcon)
 		return (-1);
 	}
 
+	/* if we have a bufferevent factory callback set, get a new bufferevent */
+	if (NULL != evcon->bufcb && 0 != (evcon->flags & EVHTTP_BEV_USED)) {
+		struct bufferevent *bev = (*evcon->bufcb)(evcon->bufcb_arg);
+
+		if (NULL == bev) {
+			event_warn("%s: bufferevent factory callback failed", __func__);
+			shutdown(evcon->fd, EVUTIL_SHUT_WR);
+			evutil_closesocket(evcon->fd);
+			evcon->fd = -1;
+			return (-1);
+		}
+        
+		if (bufferevent_get_base(bev) != evcon->base) {
+			bufferevent_base_set(evcon->base, bev);
+		}
+
+		/* ensure freeing doesn't close the newly-allocated file descriptor
+		 * (which might just have the same fd number) */
+		bufferevent_setfd(evcon->bufev, -1);
+		bufferevent_free(evcon->bufev);
+		evcon->bufev = bev;
+		evcon->flags ^= EVHTTP_BEV_USED;
+	}
+
 	/* Set up a callback for successful connection setup */
 	bufferevent_setfd(evcon->bufev, evcon->fd);
 	bufferevent_setcb(evcon->bufev,