refactors to make userdata part of the common connData struct

demo/server_demo.c

@@ -22,51 +22,51 @@ struct RequestData {
 };
 
 /** "About" page */
-httpd_cgi_state templateReplacer(TplData *td)
+httpd_cgi_state templateReplacer(HttpdConnData *conn, const char *token)
 {
     struct RequestData *rd;
 
-    if (!td->conn) {
-        if (td->userData) {
-            httpdPlatFree(td->userData);
-            td->userData = NULL;
+    if (!token) {
+        if (conn->userData) {
+            httpdPlatFree(conn->userData);
+            conn->userData = NULL;
         }
         return HTTPD_CGI_DONE;
     }
 
-    if (!td->userData) {
+    if (!conn->userData) {
         rd = httpdPlatMalloc(sizeof(struct RequestData));
-        td->userData = rd;
+        conn->userData = rd;
         rd->counter = 0;
 
         shared.visits++;
     } else {
-        rd = td->userData;
+        rd = conn->userData;
     }
 
     char buf[100];
 
-    if (streq(td->token, "visits")) {
+    if (streq(token, "visits")) {
         sprintf(buf, "%d", shared.visits);
-        tplSend(td, buf);
+        tplSend(conn, buf);
     }
-    else if (streq(td->token, "html1")) {
-        tplSend(td, "<a href=\"foo\">foo</a>");
+    else if (streq(token, "html1")) {
+        tplSend(conn, "<a href=\"foo\">foo</a>");
     }
-    else if (streq(td->token, "html2")) {
-        tplSend(td, "<a href=\"bar\">bar</a>");
+    else if (streq(token, "html2")) {
+        tplSend(conn, "<a href=\"bar\">bar</a>");
     }
-    else if (streq(td->token, "json1")) {
-        tplSend(td, "\"hello\":\"foo<angle>'' and backslash\\ \" also crlf \r\n");
+    else if (streq(token, "json1")) {
+        tplSend(conn, "\"hello\":\"foo<angle>'' and backslash\\ \" also crlf \r\n");
     }
-    else if (streq(td->token, "multipart")) {
+    else if (streq(token, "multipart")) {
         if (rd->counter < 100) {
             sprintf(buf, "HELLO %d, ", rd->counter);
-            tplSend(td, buf);
+            tplSend(conn, buf);
             rd->counter++;
             return HTTPD_CGI_MORE;
         }
-        tplSend(td, "and that's it.");
+        tplSend(conn, "and that's it.");
     }
     else {
         return HTTPD_CGI_NOTFOUND;

spritehttpd/include/cgi-espfs.h

@@ -5,25 +5,12 @@
 
 #define HTTPD_ESPFS_TOKEN_LEN 64
 
-typedef struct TplData {
-  /// Connection. Is set to NULL during final clean-up
-  HttpdConnData *conn;
-  /// Currently parsed token. Cleared during clean-up
-  const char *token;
-  /// Pointer to arbitrary user data that should be freed/cleaned up
-  /// during clean-up
-  void *userData;
-} TplData;
-
 /**
  * The template substitution callback.
- *
  * Returns CGI_MORE if more should be sent within the token, CGI_DONE otherwise.
- *
- * The td.userData pointer can be used to attach arbitrary data to the request.
- * If connData is NULL (and token empty), we are doing cleanup - free td.userData, if needed.
+ * If token is NULL, we're doing a clean-up at the end of the request, this is a good place to free any user data.
  */
-typedef httpd_cgi_state (* TplCallback)(TplData *td);
+typedef httpd_cgi_state (* TplCallback)(HttpdConnData *conn, const char *token);
 
 /**
  * Serve a static file from EspFs. The file name is in the first CGI arg. If NULL, the URI is used as a file name.
@@ -52,7 +39,7 @@ httpd_cgi_state cgiEspFsTemplate(HttpdConnData *conn);
  * @param len - string len
  * @return 1 = OK
  */
-int tplSendN(TplData *td, const char *str, int len);
+int tplSendN(HttpdConnData *conn, const char *str, int len);
 
 /**
  * Send template substitution string (use strlen)
@@ -61,6 +48,6 @@ int tplSendN(TplData *td, const char *str, int len);
  * @param str - string to send
  * @return 1 = OK
  */
-static inline int tplSend(TplData *tpd, const char *str) {
-    return tplSendN(tpd, str, strlen(str));
+static inline int tplSend(HttpdConnData *conn, const char *str) {
+    return tplSendN(conn, str, strlen(str));
 }

spritehttpd/include/cgi-websocket.h

@@ -50,8 +50,6 @@ typedef struct WebsockPriv WebsockPriv;
 // TODO convert to container_of and avoid separate malloc for priv
 
 struct Websock {
-  /// Pointer to arbitrary user data, put there e.g. during 'recvCb'
-  void *userData;
   /// Connection pointer
   HttpdConnData *conn;
   /// Receive callback - new data; optional, set by user in WsConnectedCb

spritehttpd/include/httpd-platform.h

@@ -85,21 +85,6 @@ void httpdPlatInit();
  */
 httpd_thread_handle_t *httpdPlatStart(struct httpd_options *opts);
 
-/**
- * Server task function, called by httpdPlatStart inside a thread
- *
- * @param pvParameters
- */
-void platHttpServerTask(void *pvParameters);
-
-/**
- * Posix format of the server task function
- *
- * @param pvParameters
- * @return
- */
-void *platHttpServerTaskPosix(void *pvParameters);
-
 /**
  * Wait for the server to end
  *
@@ -116,3 +101,20 @@ void httpdPlatJoin(httpd_thread_handle_t *handle);
  * @return 0 on success
  */
 int httpdPlatEspfsRead(void *dest, uint32_t offset, size_t len);
+
+/**
+ * Server task function, called by httpdPlatStart inside a thread.
+ *
+ * @internal
+ * @param pvParameters
+ */
+void httpdServerTask(void *pvParameters);
+
+/**
+ * Posix format of the server task function.
+ *
+ * @internal
+ * @param pvParameters
+ * @return
+ */
+void *httpdServerTaskPosix(void *pvParameters);

spritehttpd/include/httpd-types.h

@@ -62,6 +62,10 @@ typedef struct HttpdPostData HttpdPostData;
 typedef httpd_cgi_state (* cgiSendCallback)(HttpdConnData *connData);
 typedef httpd_cgi_state (* cgiRecvHandler)(HttpdConnData *connData, uint8_t *data, size_t len);
 
+/// Callback type that releases the user data attached to the connection.
+/// The format is compatible with regular "free()" or
+typedef void (* httpdConnUserDataCleanupCb)(void *userData);
+
 struct httpd_options {
   uint16_t port;
 };
@@ -79,15 +83,21 @@ struct HttpdConnData {
 
   cgiSendCallback cgi;	    // CGI function pointer
   cgiRecvHandler recvHdl;	// Handler for data received after headers, if any
-  const void *cgiArg;		// Argument to the CGI function, as stated as the 3rd argument of
-                            // the builtInUrls entry that referred to the CGI function.
-  const void *cgiArg2;	    // 4th argument of the builtInUrls entries, used to pass template file to the tpl handler.
+  const void *cgiArg;		// First CGI argument, can be NULL if not needed
+  const void *cgiArg2;	    // Second CGI argument for CGIs that need two parameters
   void *cgiData;            // Opaque data pointer for the CGI function
 
+  /// Opaque data pointer for user data that carries over between CGI calls for the same connection.
+  void *userData;
+  /// If userData is not NULL when a connection is finalized, this function (if set) is called to handle the cleanup.
+  /// This mechanism is useful when the user data is allocated by an auth CGI and the request can end in any number
+  /// of other routes, perhaps even with a static file - then the cleanup code doesn't need to be repeated everywhere.
+  httpdConnUserDataCleanupCb userDataCleanupCb;
+
   // this should be at the end because of padding
   uint16_t remote_port;		// Remote TCP port
   uint8_t remote_ip[4];		// IP address of client
-  uint8_t slot;				// Slot ID
+  uint8_t slot;				// Slot ID - index in s_connData
 };
 
 //A struct describing the POST data sent inside the http connection.  This is used by the CGI functions

spritehttpd/include/httpd.h

@@ -315,5 +315,7 @@ void httpdConnRelease(ConnTypePtr conn);
 /**
  * Close and retire all sockets.
  * Called during httpd shutdown.
+ *
+ * @internal
  */
 void httpdInternalCloseAllSockets();

spritehttpd/src/cgi-espfs.c

@@ -82,15 +82,15 @@ EspFsFile *tryOpenIndex(const char *path)
     return NULL; // failed to guess the right name
 }
 
-static httpd_cgi_state serveStaticFile(HttpdConnData *connData, const char *filepath)
+static httpd_cgi_state serveStaticFile(HttpdConnData *hconn, const char *filepath)
 {
-    EspFsFile *file = connData->cgiData;
+    EspFsFile *file = hconn->cgiData;
     int len;
     uint8_t buff[FILE_CHUNK_LEN + 1];
     char acceptEncodingBuffer[64 + 1];
     int isGzip;
 
-    if (connData->conn == NULL) {
+    if (hconn->conn == NULL) {
         //Connection aborted. Clean up.
         espFsClose(file);
         return HTTPD_CGI_DONE;
@@ -124,31 +124,31 @@ static httpd_cgi_state serveStaticFile(HttpdConnData *connData, const char *file
         if (isGzip) {
             // Check the browser's "Accept-Encoding" header. If the client does not
             // advertise that he accepts GZIP send a warning message (telnet users for e.g.)
-            httpdGetHeader(connData, "Accept-Encoding", acceptEncodingBuffer, 64);
+            httpdGetHeader(hconn, "Accept-Encoding", acceptEncodingBuffer, 64);
             if (strstr(acceptEncodingBuffer, "gzip") == NULL) {
                 //No Accept-Encoding: gzip header present
-                httpdSendStr(connData, gzipNonSupportedMessage);
+                httpdSendStr(hconn, gzipNonSupportedMessage);
                 espFsClose(file);
                 return HTTPD_CGI_DONE;
             }
         }
 
-        connData->cgiData = file;
-        httpdStartResponse(connData, 200);
+        hconn->cgiData = file;
+        httpdStartResponse(hconn, 200);
         const char *mime = httpdGetMimetypeOr(filepath, "application/octet-stream");
-        httpdHeader(connData, "Content-Type", mime);
+        httpdHeader(hconn, "Content-Type", mime);
         if (isGzip) {
-            httpdHeader(connData, "Content-Encoding", "gzip");
+            httpdHeader(hconn, "Content-Encoding", "gzip");
         }
-        httpdAddCacheHeaders(connData, mime);
-        httpdEndHeaders(connData);
+        httpdAddCacheHeaders(hconn, mime);
+        httpdEndHeaders(hconn);
         return HTTPD_CGI_MORE;
     }
 
     len = espFsRead(file, buff, FILE_CHUNK_LEN);
     if (len > 0) {
         espfs_dbg("[EspFS] Read file chunk: %d bytes", len);
-        httpdSend(connData, buff, len);
+        httpdSend(hconn, buff, len);
     }
     if (len != FILE_CHUNK_LEN) {
         //We're done.
@@ -182,13 +182,10 @@ typedef enum {
 typedef struct {
   EspFsFile *file;
 
-  // this is the struct passed to user
-  TplData td;
-
   int tokenPos;
-
   char buff[FILE_CHUNK_LEN + 1];
   char token[HTTPD_ESPFS_TOKEN_LEN];
+  char *pToken;
 
   bool chunk_resume;
   int buff_len;
@@ -199,17 +196,19 @@ typedef struct {
 } TplDataInternal;
 
 
-int tplSendN(TplData *td, const char *str, int len)
+int tplSendN(HttpdConnData *conn, const char *str, int len)
 {
-    if (td == NULL) { return 0; }
-    TplDataInternal *tdi = container_of(td, TplDataInternal, td);
+    if (conn == NULL) { return 0; }
+
+    TplDataInternal *tdi = conn->cgiData;
+    if (!tdi) { return 0; }
 
     if (tdi->tokEncode == ENCODE_PLAIN) {
-        return httpdSendStrN(td->conn, str, len);
+        return httpdSendStrN(conn, str, len);
     } else if (tdi->tokEncode == ENCODE_HTML) {
-        return httpdSend_html(td->conn, str, len);
+        return httpdSend_html(conn, str, len);
     } else if (tdi->tokEncode == ENCODE_JS) {
-        return httpdSend_js(td->conn, str, len);
+        return httpdSend_js(conn, str, len);
     }
     return 0;
 }
@@ -228,9 +227,7 @@ httpd_cgi_state cgiEspFsTemplate(HttpdConnData *conn)
         if (tdi) {
             TplCallback callback = (TplCallback) conn->cgiArg;
             if (callback) {
-                tdi->td.conn = NULL; // indicate that this is the final call
-                tdi->td.token = NULL;
-                callback(&tdi->td);
+                callback(conn, NULL);
             }
 
             espFsClose(tdi->file);
@@ -275,9 +272,6 @@ httpd_cgi_state cgiEspFsTemplate(HttpdConnData *conn)
             return HTTPD_CGI_NOTFOUND;
         }
 
-        tdi->td.conn = conn;
-        tdi->td.userData = NULL;
-
         tdi->tokenPos = -1;
         conn->cgiData = tdi;
         httpdStartResponse(conn, 200);
@@ -356,13 +350,13 @@ httpd_cgi_state cgiEspFsTemplate(HttpdConnData *conn)
                                 tdi->tokEncode = ENCODE_JS;
                             }
 
-                            tdi->td.token = &tdi->token[prefixLen];
+                            tdi->pToken = &tdi->token[prefixLen];
                         }
 
                         tdi->chunk_resume = false;
 
                         TplCallback callback = (TplCallback) conn->cgiArg;
-                        httpd_cgi_state status = callback(&tdi->td);
+                        httpd_cgi_state status = callback(conn, tdi->pToken);
                         if (status == HTTPD_CGI_MORE) {
 //							espfs_dbg("Multi-part tpl subst, saving parser state");
                             // wants to send more in this token's place.....
@@ -420,9 +414,7 @@ httpd_cgi_state cgiEspFsTemplate(HttpdConnData *conn)
 
         TplCallback callback = (TplCallback) conn->cgiArg;
         if (callback) {
-            tdi->td.conn = NULL;
-            tdi->td.token = NULL;
-            callback(&tdi->td);
+            callback(conn, NULL);
         }
 
         espfs_info("Template sent.");

spritehttpd/src/httpd-loop.c

@@ -47,7 +47,7 @@ void httpdConnDisconnect(ConnTypePtr conn)
     conn->needWriteDoneNotif = 1; //because the real close is done in the writable select code
 }
 
-void platHttpServerTask(void *pvParameters)
+void httpdServerTask(void *pvParameters)
 {
     int32_t listenfd;
     int32_t len;
@@ -81,7 +81,7 @@ void platHttpServerTask(void *pvParameters)
     do {
         listenfd = socket(AF_INET, SOCK_STREAM, 0);
         if (listenfd == -1) {
-            error("platHttpServerTask: failed to create sock!");
+            error("httpdServerTask: failed to create sock!");
             httpdPlatDelayMs(1000);
         }
     } while (listenfd == -1);
@@ -99,7 +99,7 @@ void platHttpServerTask(void *pvParameters)
     do {
         ret = bind(listenfd, (struct sockaddr *) &server_addr, sizeof(server_addr));
         if (ret != 0) {
-            error("platHttpServerTask: failed to bind!");
+            error("httpdServerTask: failed to bind!");
             httpdPlatDelayMs(1000);
         }
     } while (ret != 0);
@@ -108,7 +108,7 @@ void platHttpServerTask(void *pvParameters)
         /* Listen to the local connection */
         ret = listen(listenfd, HTTPD_MAX_CONNECTIONS);
         if (ret != 0) {
-            error("platHttpServerTask: failed to listen!");
+            error("httpdServerTask: failed to listen!");
             httpdPlatDelayMs(1000);
         }
     } while (ret != 0);
@@ -162,7 +162,7 @@ void platHttpServerTask(void *pvParameters)
                 len = sizeof(struct sockaddr_in);
                 const int remotefd = accept(listenfd, (struct sockaddr *) &remote_addr, (socklen_t *) &len);
                 if (remotefd < 0) {
-                    warn("platHttpServerTask: Huh? Accept failed.");
+                    warn("httpdServerTask: Huh? Accept failed.");
                     continue;
                 }
 
@@ -174,7 +174,7 @@ void platHttpServerTask(void *pvParameters)
                     }
                 }
                 if (socknum >= HTTPD_MAX_CONNECTIONS) {
-                    warn("platHttpServerTask: Huh? Got accept with all slots full.");
+                    warn("httpdServerTask: Huh? Got accept with all slots full.");
                     continue;
                 }
 

spritehttpd/src/httpd.c

@@ -18,7 +18,8 @@ Esp8266 http server - core routines
 #include "httpd-utils.h"
 #include "httpd-logging.h"
 
-static void httpdRetireConn(HttpdConnData *conn);
+static void cleanupCgiAndUserData(HttpdConnData *hconn);
+static void httpdRetireConn(HttpdConnData *hconn);
 
 //This gets set at init time.
 static const HttpdBuiltInUrl *s_builtInUrls;
@@ -128,27 +129,34 @@ static HttpdConnData *httpdFindConnData(ConnTypePtr conn, const char *remIp, int
 }
 
 //Retires a connection for re-use
-static void httpdRetireConn(HttpdConnData *conn)
+static void httpdRetireConn(HttpdConnData *hconn)
 {
-    if (!conn) {
+    if (!hconn) {
         return;
     }
-    if (conn->priv->sendBacklog != NULL) {
+    http_info("Pool slot %d: socket closed.", hconn->slot);
+
+    // this sets the `conn` pointer to NULL to indicate we are cleaning up to CGI
+    cleanupCgiAndUserData(hconn);
+
+    // Free any memory allocated for backlog - walk the linked list
+    if (hconn->priv->sendBacklog != NULL) {
         HttpSendBacklogItem *i, *j;
-        i = conn->priv->sendBacklog;
+        i = hconn->priv->sendBacklog;
         do {
             j = i;
             i = i->next;
             httpdPlatFree(j);
         } while (i != NULL);
     }
-    if (conn->post->buff != NULL) { httpdPlatFree(conn->post->buff); }
-    if (conn->post != NULL) { httpdPlatFree(conn->post); }
-    if (conn->priv != NULL) { httpdPlatFree(conn->priv); }
-    httpdPlatFree(conn);
-    for (int i = 0; i < HTTPD_MAX_CONNECTIONS; i++) {
-        if (s_connData[i] == conn) { s_connData[i] = NULL; }
-    }
+    if (hconn->post->buff != NULL) { httpdPlatFree(hconn->post->buff); }
+    if (hconn->post != NULL) { httpdPlatFree(hconn->post); }
+    if (hconn->priv != NULL) { httpdPlatFree(hconn->priv); }
+
+    // Unlink from the connection list
+    s_connData[hconn->slot] = NULL;
+    // release memory
+    httpdPlatFree(hconn);
 }
 
 //Get the value of a certain header in the HTTP client head
@@ -525,12 +533,12 @@ static void httpdProcessRequest(HttpdConnData *conn)
             //See if there's a literal match
             if (streq(route, conn->url)) { match = 1; }
             //See if there's a wildcard match (*)
-            if (last_char(route) == '*' &&
+            if (!match && last_char(route) == '*' &&
                 strneq(route, conn->url, strlen(route) - 1)) {
                 match = 1;
             }
             // Optional slash (/?)
-            if (last_char(route) == '?' && last_char_n(route, 2) == '/' &&
+            if (!match && last_char(route) == '?' && last_char_n(route, 2) == '/' &&
                 strneq(route, conn->url, strlen(route) - 2) &&
                 strlen(conn->url) <= strlen(route) - 1) {
                 match = 1;
@@ -820,13 +828,30 @@ void httpdDisconCb(ConnTypePtr rconn, const char *remIp, int remPort)
         httpdPlatUnlock();
         return;
     }
-    http_info("Pool slot %d: socket closed.", hconn->slot);
-    hconn->conn = NULL; //indicate cgi the connection is gone
-    if (hconn->cgi) { hconn->cgi(hconn); } //Execute cgi fn if needed
     httpdRetireConn(hconn);
     httpdPlatUnlock();
 }
 
+/**
+ * Clean up cgiData and userData and do any cgi-specific finalizing.
+ *
+ * @param hconn
+ */
+static void cleanupCgiAndUserData(HttpdConnData *hconn)
+{
+    hconn->conn = NULL; //indicate cgi the connection is gone
+    if (hconn->cgi) {
+        //Execute cgi fn if needed
+        hconn->cgi(hconn);
+        hconn->cgi = NULL;
+    }
+    if (hconn->userDataCleanupCb && hconn->userData) {
+        hconn->userDataCleanupCb(hconn->userData);
+        hconn->userData = NULL;
+        hconn->userDataCleanupCb = NULL;
+    }
+}
+
 
 int httpdConnectCb(ConnTypePtr conn, const char *remIp, int remPort)
 {

spritehttpd/src/port/httpd-posix.c

@@ -45,9 +45,9 @@ struct httpd_thread_handle {
     // TODO some way to signal shutdown?
 };
 
-void* platHttpServerTaskPosix(void *pvParameters)
+void* httpdServerTaskPosix(void *pvParameters)
 {
-    platHttpServerTask(pvParameters);
+    httpdServerTask(pvParameters);
     return NULL;
 }
 
@@ -59,7 +59,7 @@ httpd_thread_handle_t *httpdPlatStart(struct httpd_options *opts)
         return NULL;
     }
 
-    pthread_create( &handle->handle, NULL, platHttpServerTaskPosix, (void*) opts);
+    pthread_create(&handle->handle, NULL, httpdServerTaskPosix, (void *) opts);
     return handle;
 }