[httpd] Refactor http handlers so they are initialized centrally

Means regex.h doesn't need to be in httpd.h making it easier to e.g. experiment
with evhtp.

src/httpd.c

@@ -42,6 +42,8 @@
 #include <event2/event.h>
 #include <event2/http.h>
 #include <event2/http_struct.h>
+
+#include <regex.h>
 #include <zlib.h>
 
 #include "logger.h"
@@ -945,8 +947,8 @@ httpd_request_parse(struct evhttp_request *req, struct httpd_uri_parsed *uri_par
   struct httpd_request *hreq;
   struct evhttp_connection *evcon;
   struct evkeyvalq *headers;
+  struct httpd_uri_map *uri;
   int req_method;
-  int i;
   int ret;
 
   CHECK_NULL(L_HTTPD, hreq = calloc(1, sizeof(struct httpd_request)));
@@ -975,26 +977,69 @@ httpd_request_parse(struct evhttp_request *req, struct httpd_uri_parsed *uri_par
     hreq->user_agent = user_agent;
 
   // Find a handler for the path
-  for (i = 0; uri_map[i].handler; i++)
+  for (uri = uri_map; uri->handler; uri++)
     {
       // Check if handler supports the current http request method
-      if (uri_map[i].method && req_method && !(req_method & uri_map[i].method))
+      if (uri->method && req_method && !(req_method & uri->method))
 	continue;
 
-      ret = regexec(&uri_map[i].preg, uri_parsed->path, 0, NULL, 0);
+      ret = regexec(uri->preg, uri_parsed->path, 0, NULL, 0);
-      if (ret == 0)
+      if (ret != 0)
-        {
+	continue;
-          hreq->handler = uri_map[i].handler;
+
-          return hreq; // Success
+      hreq->handler = uri->handler;
-        }
+      return hreq; // Success
     }
 
   // Handler not found, that's an error
   free(hreq);
-
   return NULL;
 }
 
+int
+httpd_handlers_set(struct httpd_uri_map *uri_map)
+{
+  struct httpd_uri_map *uri;
+  char buf[64];
+  int ret;
+
+  for (uri = uri_map; uri->handler; uri++)
+    {
+      uri->preg = calloc(1, sizeof(regex_t));
+      if (!uri->preg)
+	{
+	  DPRINTF(E_LOG, L_HTTPD, "Error setting URI handler, out of memory");
+	  goto error;
+	}
+
+      ret = regcomp(uri->preg, uri->regexp, REG_EXTENDED | REG_NOSUB);
+      if (ret != 0)
+	{
+	  regerror(ret, uri->preg, buf, sizeof(buf));
+	  DPRINTF(E_LOG, L_HTTPD, "Error setting URI handler, regexp error: %s\n", buf);
+	  goto error;
+	}
+    }
+
+  return 0;
+
+ error:
+  httpd_handlers_unset(uri_map);
+  return -1;
+}
+
+void
+httpd_handlers_unset(struct httpd_uri_map *uri_map)
+{
+  struct httpd_uri_map *uri;
+
+  for (uri = uri_map; uri->preg; uri++)
+    {
+      regfree(uri->preg); // Frees allocation by regcomp
+      free(uri->preg); // Frees our own calloc
+    }
+}
+
 /* Thread: httpd */
 void
 httpd_stream_file(struct evhttp_request *req, int id)

src/httpd.h

@@ -3,7 +3,6 @@
 #define __HTTPD_H__
 
 #include <stdbool.h>
-#include <regex.h>
 #include <time.h>
 #include <event2/http.h>
 #include <event2/buffer.h>
@@ -72,7 +71,7 @@ struct httpd_uri_map
   int method;
   char *regexp;
   int (*handler)(struct httpd_request *hreq);
-  regex_t preg;
+  void *preg;
 };
 
 /*
@@ -97,6 +96,12 @@ httpd_uri_parse(const char *uri);
 struct httpd_request *
 httpd_request_parse(struct evhttp_request *req, struct httpd_uri_parsed *uri_parsed, const char *user_agent, struct httpd_uri_map *uri_map);
 
+int
+httpd_handlers_set(struct httpd_uri_map *uri_map);
+
+void
+httpd_handlers_unset(struct httpd_uri_map *uri_map);
+
 void
 httpd_stream_file(struct evhttp_request *req, int id);
 

src/httpd_artworkapi.c

@@ -211,21 +211,7 @@ artworkapi_is_request(const char *path)
 int
 artworkapi_init(void)
 {
-  char buf[64];
+  CHECK_ERR(L_WEB, httpd_handlers_set(artworkapi_handlers));
-  int i;
-  int ret;
-
-  for (i = 0; artworkapi_handlers[i].handler; i++)
-    {
-      ret = regcomp(&artworkapi_handlers[i].preg, artworkapi_handlers[i].regexp, REG_EXTENDED | REG_NOSUB);
-      if (ret != 0)
-	{
-	  regerror(ret, &artworkapi_handlers[i].preg, buf, sizeof(buf));
-
-	  DPRINTF(E_FATAL, L_WEB, "artwork api init failed; regexp error: %s\n", buf);
-	  return -1;
-	}
-    }
 
   return 0;
 }
@@ -233,8 +219,5 @@ artworkapi_init(void)
 void
 artworkapi_deinit(void)
 {
-  int i;
+  httpd_handlers_unset(artworkapi_handlers);
-
-  for (i = 0; artworkapi_handlers[i].handler; i++)
-    regfree(&artworkapi_handlers[i].preg);
 }

src/httpd_daap.c

@@ -42,7 +42,6 @@
 #include <unistd.h>
 
 #include <event2/event.h>
-#include <event2/bufferevent.h>
 
 #include "httpd_daap.h"
 #include "logger.h"
@@ -993,7 +992,6 @@ daap_reply_update(struct httpd_request *hreq)
 {
   struct daap_update_request *ur;
   struct evhttp_connection *evcon;
-  struct bufferevent *bufev;
   const char *param;
   int reqd_rev;
   int ret;
@@ -1073,18 +1071,7 @@ daap_reply_update(struct httpd_request *hreq)
    */
   evcon = evhttp_request_get_connection(hreq->req);
   if (evcon)
-    {
+    evhttp_connection_set_closecb(evcon, update_fail_cb, ur);
-      evhttp_connection_set_closecb(evcon, update_fail_cb, ur);
-
-      // This is a workaround for some versions of libevent (2.0, but possibly
-      // also 2.1) that don't detect if the client hangs up, and thus don't
-      // clean up and never call update_fail_cb(). See github issue #870 and
-      // https://github.com/libevent/libevent/issues/666. It should probably be
-      // removed again in the future. The workaround is also present in dacp.c
-      bufev = evhttp_connection_get_bufferevent(evcon);
-      if (bufev)
-	bufferevent_enable(bufev, EV_READ);
-    }
 
   return DAAP_REPLY_NONE;
 }
@@ -2424,25 +2411,11 @@ daap_reply_build(const char *uri, const char *user_agent, int is_remote)
 int
 daap_init(void)
 {
-  char buf[64];
-  int i;
-  int ret;
-
   srand((unsigned)time(NULL));
   current_rev = 2;
   update_requests = NULL;
 
-  for (i = 0; daap_handlers[i].handler; i++)
+  CHECK_ERR(L_DAAP, httpd_handlers_set(daap_handlers));
-    {
-      ret = regcomp(&daap_handlers[i].preg, daap_handlers[i].regexp, REG_EXTENDED | REG_NOSUB);
-      if (ret != 0)
-        {
-          regerror(ret, &daap_handlers[i].preg, buf, sizeof(buf));
-
-          DPRINTF(E_FATAL, L_DAAP, "DAAP init failed; regexp error: %s\n", buf);
-	  return -1;
-        }
-    }
 
   return 0;
 }
@@ -2453,10 +2426,6 @@ daap_deinit(void)
   struct daap_session *s;
   struct daap_update_request *ur;
   struct evhttp_connection *evcon;
-  int i;
-
-  for (i = 0; daap_handlers[i].handler; i++)
-    regfree(&daap_handlers[i].preg);
 
   for (s = daap_sessions; daap_sessions; s = daap_sessions)
     {
@@ -2477,4 +2446,6 @@ daap_deinit(void)
 
       update_free(ur);
     }
+
+  httpd_handlers_unset(daap_handlers);
 }

src/httpd_dacp.c

@@ -36,7 +36,6 @@
 #endif
 
 #include <event2/event.h>
-#include <event2/bufferevent.h>
 
 #include "httpd_dacp.h"
 #include "httpd_daap.h"
@@ -2236,7 +2235,6 @@ dacp_reply_playstatusupdate(struct httpd_request *hreq)
 {
   struct dacp_update_request *ur;
   struct evhttp_connection *evcon;
-  struct bufferevent *bufev;
   const char *param;
   int reqd_rev;
   int ret;
@@ -2297,18 +2295,7 @@ dacp_reply_playstatusupdate(struct httpd_request *hreq)
    */
   evcon = evhttp_request_get_connection(hreq->req);
   if (evcon)
-    {
+    evhttp_connection_set_closecb(evcon, update_fail_cb, ur);
-      evhttp_connection_set_closecb(evcon, update_fail_cb, ur);
-
-      // This is a workaround for some versions of libevent (2.0, but possibly
-      // also 2.1) that don't detect if the client hangs up, and thus don't
-      // clean up and never call update_fail_cb(). See github issue #870 and
-      // https://github.com/libevent/libevent/issues/666. It should probably be
-      // removed again in the future. The workaround is also present in daap.c
-      bufev = evhttp_connection_get_bufferevent(evcon);
-      if (bufev)
-	bufferevent_enable(bufev, EV_READ);
-    }
 
   return 0;
 }
@@ -2895,12 +2882,7 @@ dacp_is_request(const char *path)
 int
 dacp_init(void)
 {
-  char buf[64];
-  int i;
-  int ret;
-
   current_rev = 2;
-  update_requests = NULL;
 
   dummy_mfi.id = DB_MEDIA_FILE_NON_PERSISTENT_ID;
   dummy_mfi.title = CFG_NAME_UNKNOWN_TITLE;
@@ -2914,72 +2896,42 @@ dacp_init(void)
   dummy_queue_item.album = CFG_NAME_UNKNOWN_ALBUM;
   dummy_queue_item.genre = CFG_NAME_UNKNOWN_GENRE;
 
+  CHECK_ERR(L_DACP, httpd_handlers_set(dacp_handlers));
+
 #ifdef HAVE_EVENTFD
   update_efd = eventfd(0, EFD_CLOEXEC);
   if (update_efd < 0)
     {
       DPRINTF(E_LOG, L_DACP, "Could not create update eventfd: %s\n", strerror(errno));
-
+      goto error;
-      return -1;
     }
+
+  CHECK_NULL(L_DACP, updateev = event_new(evbase_httpd, update_efd, EV_READ, playstatusupdate_cb, NULL));
 #else
 # ifdef HAVE_PIPE2
-  ret = pipe2(update_pipe, O_CLOEXEC);
+  int ret = pipe2(update_pipe, O_CLOEXEC);
 # else
-  ret = pipe(update_pipe);
+  int ret = pipe(update_pipe);
 # endif
   if (ret < 0)
     {
       DPRINTF(E_LOG, L_DACP, "Could not create update pipe: %s\n", strerror(errno));
-
+      goto error;
-      return -1;
     }
+
+  CHECK_NULL(L_DACP, updateev = event_new(evbase_httpd, update_pipe[0], EV_READ, playstatusupdate_cb, NULL));
 #endif /* HAVE_EVENTFD */
 
-  for (i = 0; dacp_handlers[i].handler; i++)
-    {
-      ret = regcomp(&dacp_handlers[i].preg, dacp_handlers[i].regexp, REG_EXTENDED | REG_NOSUB);
-      if (ret != 0)
-        {
-          regerror(ret, &dacp_handlers[i].preg, buf, sizeof(buf));
-
-          DPRINTF(E_FATAL, L_DACP, "DACP init failed; regexp error: %s\n", buf);
-	  goto regexp_fail;
-        }
-    }
-
-#ifdef HAVE_EVENTFD
-  updateev = event_new(evbase_httpd, update_efd, EV_READ, playstatusupdate_cb, NULL);
-#else
-  updateev = event_new(evbase_httpd, update_pipe[0], EV_READ, playstatusupdate_cb, NULL);
-#endif
-  if (!updateev)
-    {
-      DPRINTF(E_LOG, L_DACP, "Could not create update event\n");
-
-      return -1;
-    }
   event_add(updateev, NULL);
 
-  seek_timer = evtimer_new(evbase_httpd, seek_timer_cb, NULL);
+  CHECK_NULL(L_DACP, seek_timer = evtimer_new(evbase_httpd, seek_timer_cb, NULL));
-  if (!seek_timer)
-    {
-      DPRINTF(E_LOG, L_DACP, "Could not create seek_timer event\n");
-
-      return -1;
-    }
 
   listener_add(dacp_playstatus_update_handler, LISTENER_PLAYER | LISTENER_VOLUME | LISTENER_QUEUE);
 
   return 0;
 
- regexp_fail:
+ error:
-#ifdef HAVE_EVENTFD
+  dacp_deinit();
-  close(update_efd);
-#else
-  close(update_pipe[0]);
-  close(update_pipe[1]);
-#endif
   return -1;
 }
 
@@ -2988,14 +2940,6 @@ dacp_deinit(void)
 {
   struct dacp_update_request *ur;
   struct evhttp_connection *evcon;
-  int i;
-
-  listener_remove(dacp_playstatus_update_handler);
-
-  event_free(seek_timer);
-
-  for (i = 0; dacp_handlers[i].handler; i++)
-    regfree(&dacp_handlers[i].preg);
 
   for (ur = update_requests; update_requests; ur = update_requests)
     {
@@ -3011,7 +2955,13 @@ dacp_deinit(void)
       free(ur);
     }
 
-  event_free(updateev);
+  listener_remove(dacp_playstatus_update_handler);
+
+  if (seek_timer)
+   event_free(seek_timer);
+
+  if (updateev)
+    event_free(updateev);
 
 #ifdef HAVE_EVENTFD
   close(update_efd);
@@ -3019,4 +2969,6 @@ dacp_deinit(void)
   close(update_pipe[0]);
   close(update_pipe[1]);
 #endif
+
+  httpd_handlers_unset(dacp_handlers);
 }

src/httpd_jsonapi.c

@@ -4794,22 +4794,9 @@ jsonapi_is_request(const char *path)
 int
 jsonapi_init(void)
 {
-  char buf[64];
   char *temp_path;
-  int i;
-  int ret;
 
-  for (i = 0; adm_handlers[i].handler; i++)
+  CHECK_ERR(L_WEB, httpd_handlers_set(adm_handlers));
-    {
-      ret = regcomp(&adm_handlers[i].preg, adm_handlers[i].regexp, REG_EXTENDED | REG_NOSUB);
-      if (ret != 0)
-	{
-	  regerror(ret, &adm_handlers[i].preg, buf, sizeof(buf));
-
-	  DPRINTF(E_FATAL, L_WEB, "JSON api init failed; regexp error: %s\n", buf);
-	  return -1;
-	}
-    }
 
   default_playlist_directory = NULL;
   allow_modifying_stored_playlists = cfg_getbool(cfg_getsec(cfg, "library"), "allow_modifying_stored_playlists");
@@ -4840,10 +4827,7 @@ jsonapi_init(void)
 void
 jsonapi_deinit(void)
 {
-  int i;
-
-  for (i = 0; adm_handlers[i].handler; i++)
-    regfree(&adm_handlers[i].preg);
-
   free(default_playlist_directory);
+
+  httpd_handlers_unset(adm_handlers);
 }

src/httpd_oauth.c

@@ -125,21 +125,7 @@ oauth_is_request(const char *path)
 int
 oauth_init(void)
 {
-  char buf[64];
+  CHECK_ERR(L_WEB, httpd_handlers_set(oauth_handlers));
-  int i;
-  int ret;
-
-  for (i = 0; oauth_handlers[i].handler; i++)
-    {
-      ret = regcomp(&oauth_handlers[i].preg, oauth_handlers[i].regexp, REG_EXTENDED | REG_NOSUB);
-      if (ret != 0)
-        {
-          regerror(ret, &oauth_handlers[i].preg, buf, sizeof(buf));
-
-          DPRINTF(E_FATAL, L_WEB, "OAuth init failed; regexp error: %s\n", buf);
-	  return -1;
-        }
-    }
 
   return 0;
 }
@@ -147,8 +133,5 @@ oauth_init(void)
 void
 oauth_deinit(void)
 {
-  int i;
+  httpd_handlers_unset(oauth_handlers);
-
-  for (i = 0; oauth_handlers[i].handler; i++)
-    regfree(&oauth_handlers[i].preg);
 }

src/httpd_rsp.c

@@ -905,23 +905,9 @@ rsp_is_request(const char *path)
 int
 rsp_init(void)
 {
-  char buf[64];
-  int i;
-  int ret;
-
   snprintf(rsp_filter_files, sizeof(rsp_filter_files), "f.data_kind = %d", DATA_KIND_FILE);
 
-  for (i = 0; rsp_handlers[i].handler; i++)
+  CHECK_ERR(L_RSP, httpd_handlers_set(rsp_handlers));
-    {
-      ret = regcomp(&rsp_handlers[i].preg, rsp_handlers[i].regexp, REG_EXTENDED | REG_NOSUB);
-      if (ret != 0)
-        {
-          regerror(ret, &rsp_handlers[i].preg, buf, sizeof(buf));
-
-          DPRINTF(E_FATAL, L_RSP, "RSP init failed; regexp error: %s\n", buf);
-	  return -1;
-        }
-    }
 
   return 0;
 }
@@ -929,8 +915,5 @@ rsp_init(void)
 void
 rsp_deinit(void)
 {
-  int i;
+  httpd_handlers_unset(rsp_handlers);
-
-  for (i = 0; rsp_handlers[i].handler; i++)
-    regfree(&rsp_handlers[i].preg);
 }