From 59e2223ef72d3a6b05f28e4b22ea4a4b83fd4be3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Hru=C5=A1ka?= Date: Fri, 27 Jan 2023 22:50:15 +0100 Subject: [PATCH] refactors to make userdata part of the common connData struct --- demo/server_demo.c | 38 +++++++++--------- spritehttpd/include/cgi-espfs.h | 23 +++-------- spritehttpd/include/cgi-websocket.h | 2 - spritehttpd/include/httpd-platform.h | 32 ++++++++------- spritehttpd/include/httpd-types.h | 20 +++++++--- spritehttpd/include/httpd.h | 2 + spritehttpd/src/cgi-espfs.c | 58 ++++++++++++--------------- spritehttpd/src/httpd-loop.c | 12 +++--- spritehttpd/src/httpd.c | 59 ++++++++++++++++++++-------- spritehttpd/src/port/httpd-posix.c | 6 +-- 10 files changed, 134 insertions(+), 118 deletions(-) diff --git a/demo/server_demo.c b/demo/server_demo.c index c77d335..fbde417 100644 --- a/demo/server_demo.c +++ b/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, "foo"); + else if (streq(token, "html1")) { + tplSend(conn, "foo"); } - else if (streq(td->token, "html2")) { - tplSend(td, "bar"); + else if (streq(token, "html2")) { + tplSend(conn, "bar"); } - else if (streq(td->token, "json1")) { - tplSend(td, "\"hello\":\"foo'' and backslash\\ \" also crlf \r\n"); + else if (streq(token, "json1")) { + tplSend(conn, "\"hello\":\"foo'' 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; diff --git a/spritehttpd/include/cgi-espfs.h b/spritehttpd/include/cgi-espfs.h index ccab51a..5944a78 100644 --- a/spritehttpd/include/cgi-espfs.h +++ b/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)); } diff --git a/spritehttpd/include/cgi-websocket.h b/spritehttpd/include/cgi-websocket.h index dbce230..b9846df 100644 --- a/spritehttpd/include/cgi-websocket.h +++ b/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 diff --git a/spritehttpd/include/httpd-platform.h b/spritehttpd/include/httpd-platform.h index c3f2c3f..2d86e70 100644 --- a/spritehttpd/include/httpd-platform.h +++ b/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); diff --git a/spritehttpd/include/httpd-types.h b/spritehttpd/include/httpd-types.h index 1e0082a..1a411db 100644 --- a/spritehttpd/include/httpd-types.h +++ b/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. - void *cgiData; // Opaque data pointer for the CGI function + 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 diff --git a/spritehttpd/include/httpd.h b/spritehttpd/include/httpd.h index d929a07..38a8635 100644 --- a/spritehttpd/include/httpd.h +++ b/spritehttpd/include/httpd.h @@ -315,5 +315,7 @@ void httpdConnRelease(ConnTypePtr conn); /** * Close and retire all sockets. * Called during httpd shutdown. + * + * @internal */ void httpdInternalCloseAllSockets(); diff --git a/spritehttpd/src/cgi-espfs.c b/spritehttpd/src/cgi-espfs.c index 150cac9..e116aa6 100644 --- a/spritehttpd/src/cgi-espfs.c +++ b/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."); diff --git a/spritehttpd/src/httpd-loop.c b/spritehttpd/src/httpd-loop.c index 2ca2c2a..8bf8e39 100644 --- a/spritehttpd/src/httpd-loop.c +++ b/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; } diff --git a/spritehttpd/src/httpd.c b/spritehttpd/src/httpd.c index 798ab6a..919f359 100644 --- a/spritehttpd/src/httpd.c +++ b/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) { diff --git a/spritehttpd/src/port/httpd-posix.c b/spritehttpd/src/port/httpd-posix.c index 9864e2c..bdd3010 100644 --- a/spritehttpd/src/port/httpd-posix.c +++ b/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; }