Addressed review comments

danielinux · danielinux · commit 3f1cec6fe816 · 2025-11-03T17:34:03.000+01:00
diff --git a/Makefile b/Makefile
@@ -1,6 +1,6 @@
 CC?=gcc
 CFLAGS:=-Wall -Werror -Wextra -I. -D_GNU_SOURCE
-CFLAGS+=-g -ggdb
+CFLAGS+=-g -ggdb -Wdeclaration-after-statement
 LDFLAGS+=-pthread
 
 CPPCHECK=cppcheck
diff --git a/src/http/httpd.c b/src/http/httpd.c
@@ -21,6 +21,7 @@
 #include "wolfip.h"
 #include "httpd.h"
 #include <ctype.h>
+#include <string.h>
 
 static const char *http_status_text(int status_code) {
     switch (status_code) {
@@ -187,7 +188,6 @@ void http_send_response_chunk_end(struct http_client *hc) {
             hc->ssl = NULL;
             wolfIP_sock_close(hc->httpd->ipstack, hc->client_sd);
             hc->client_sd = 0;
-            return;
         }
     } else {
         if (wolfIP_sock_send(hc->httpd->ipstack, hc->client_sd, "0\r\n\r\n", 5, 0) <= 0) {
@@ -217,27 +217,47 @@ void http_send_418_teapot(struct http_client *hc) {
             http_status_text(HTTP_STATUS_TEAPOT), "text/plain", 0);
 }
 
-int http_url_decode(char *buf, size_t len) {
+static int http_hex_value(char c)
+{
+    if (c >= '0' && c <= '9')
+        return c - '0';
+    c = (char)tolower((unsigned char)c);
+    if (c >= 'a' && c <= 'f')
+        return 10 + (c - 'a');
+    return -1;
+}
+
+int http_url_decode(char *buf, size_t len)
+{
     char *p = buf;
-    char *q;
-    while (p < buf + len) {
-        q = strchr(p, '%');
-        if (!q) {
-            break;
-        }
-        /* Ensure we have two more hex digits */
-        if (q + 2 >= buf + len) {
-            break; /* Malformed escape */
-        }
-        /* Validate hex characters before conversion */
-        if (!isxdigit((unsigned char)q[1]) || !isxdigit((unsigned char)q[2])) {
+    char *end = buf + len;
+    int hi;
+    int lo;
+    size_t tail;
+
+    while (p < end) {
+        char *percent = memchr(p, '%', (size_t)(end - p));
+        if (!percent)
             break;
-        }
-        *q = (char) strtol(q + 1, NULL, 16);
-        memmove(q + 1, q + 3, len - (q + 3 - buf));
+
+        if (percent + 2 >= end)
+            return HTTP_URL_DECODE_ERR_TRUNCATED;
+
+        hi = http_hex_value(percent[1]);
+        lo = http_hex_value(percent[2]);
+        if (hi < 0 || lo < 0)
+            return HTTP_URL_DECODE_ERR_BAD_ESCAPE;
+
+        *percent = (char)((hi << 4) | lo);
+
+        tail = (size_t)(end - (percent + 3));
+        memmove(percent + 1, percent + 3, tail);
+        end -= 2;
         len -= 2;
+        p = percent + 1;
     }
-    return len;
+
+    return (int)len;
 }
 
 int http_url_encode(char *buf, size_t len, size_t max_len) {
@@ -258,8 +278,11 @@ int http_url_encode(char *buf, size_t len, size_t max_len) {
         *(q + 2) = '0';
         len += 2;
     }
-    if (q && (len < max_len))
+    if (q) {
+        if (len >= max_len)
+            return -1; /* No space for the null terminator */
         q[len] = '\0';
+    }
     return len;
 }
 
@@ -269,10 +292,18 @@ static int parse_http_request(struct http_client *hc, uint8_t *buf, size_t len)
     char *q;
     size_t n;
     int ret;
+    int decoded_len;
     struct http_request req;
     struct http_url *url = NULL;
     memset(&req, 0, sizeof(struct http_request));
-    http_url_decode(p, len); /* Decode can be done in place */ 
+    decoded_len = http_url_decode(p, len); /* Decode can be done in place */
+    if (decoded_len < 0) {
+        http_send_response_headers(hc, HTTP_STATUS_BAD_REQUEST,
+                http_status_text(HTTP_STATUS_BAD_REQUEST), "text/plain", 0);
+        return decoded_len;
+    }
+    len = (size_t)decoded_len;
+    end = p + len;
     if (len < 4)
         goto bad_request;
     /* Parse the request line */
diff --git a/src/http/httpd.h b/src/http/httpd.h
@@ -78,6 +78,11 @@ void http_send_500_server_error(struct http_client *hc);
 void http_send_503_service_unavailable(struct http_client *hc);
 void http_send_418_teapot(struct http_client *hc);
 
+/* URL decoding return codes */
+#define HTTP_URL_DECODE_ERR_TRUNCATED  (-1)
+#define HTTP_URL_DECODE_ERR_BAD_ESCAPE (-2)
+
+/* Returns the decoded length on success, or negative error code on failure. */
 int http_url_decode(char *buf, size_t len);
 int http_url_encode(char *buf, size_t len, size_t max_len);
 
diff --git a/src/port/posix/bsd_socket.c b/src/port/posix/bsd_socket.c
@@ -153,10 +153,10 @@ int wolfIP_sock_fcntl(struct wolfIP *ipstack, int fd, int cmd, int arg) {
 int fcntl(int fd, int cmd, ...) {
     va_list ap;
     int arg;
+    int ret;
     va_start(ap, cmd);
     arg = va_arg(ap, int);
     va_end(ap);
-    int ret;
     if (in_the_stack) {
         return host_fcntl(fd, cmd, arg);
     } else {
diff --git a/src/port/posix/linux_tap.c b/src/port/posix/linux_tap.c
@@ -48,8 +48,8 @@ void print_buffer(uint8_t *buf, int len)
 static int tap_poll(struct wolfIP_ll_dev *ll, void *buf, uint32_t len)
 {
     struct pollfd pfd;
-    (void)ll;
     int ret;
+    (void)ll;
     pfd.fd = tap_fd;
     pfd.events = POLLIN;
     ret = poll(&pfd, 1, 2);
diff --git a/src/port/wolfssl_io.c b/src/port/wolfssl_io.c
@@ -24,7 +24,14 @@
 #include <wolfssl/ssl.h>
 #include <wolfssl/wolfcrypt/memory.h>
 
-#define MAX_WOLFIP_CTX 8
+#ifndef EAGAIN
+#define EAGAIN (11)
+#endif
+
+
+#ifndef MAX_WOLFIP_CTX
+    #define MAX_WOLFIP_CTX 8 /* Default value */
+#endif
 
 struct ctx_entry {
     WOLFSSL_CTX *ctx;
@@ -62,13 +69,14 @@ static void wolfIP_register_stack(WOLFSSL_CTX *ctx, struct wolfIP *stack)
 static int wolfIP_io_recv(WOLFSSL* ssl, char* buf, int sz, void* ctx)
 {
     struct wolfip_io_desc *desc = (struct wolfip_io_desc *)ctx;
+    int ret;
     (void)ssl;
 
     if (!desc || !desc->stack)
         return WOLFSSL_CBIO_ERR_GENERAL;
 
-    int ret = wolfIP_sock_recv(desc->stack, desc->fd, buf, sz, 0);
-    if (ret == -11 || ret == -1)
+    ret = wolfIP_sock_recv(desc->stack, desc->fd, buf, sz, 0);
+    if (ret == -EAGAIN || ret == -1)
         return WOLFSSL_CBIO_ERR_WANT_READ;
     if (ret <= 0)
         return WOLFSSL_CBIO_ERR_CONN_CLOSE;
@@ -78,13 +86,14 @@ static int wolfIP_io_recv(WOLFSSL* ssl, char* buf, int sz, void* ctx)
 static int wolfIP_io_send(WOLFSSL* ssl, char* buf, int sz, void* ctx)
 {
     struct wolfip_io_desc *desc = (struct wolfip_io_desc *)ctx;
+    int ret;
     (void)ssl;
 
     if (!desc || !desc->stack)
         return WOLFSSL_CBIO_ERR_GENERAL;
 
-    int ret = wolfIP_sock_send(desc->stack, desc->fd, buf, sz, 0);
-    if (ret == -11 || ret == -1)
+    ret = wolfIP_sock_send(desc->stack, desc->fd, buf, sz, 0);
+    if (ret == -EAGAIN || ret == -1)
         return WOLFSSL_CBIO_ERR_WANT_WRITE;
     if (ret <= 0)
         return WOLFSSL_CBIO_ERR_CONN_CLOSE;
@@ -101,8 +110,20 @@ int wolfSSL_SetIO_wolfIP_CTX(WOLFSSL_CTX* ctx, struct wolfIP *s)
 
 int wolfSSL_SetIO_wolfIP(WOLFSSL* ssl, int fd)
 {
-    WOLFSSL_CTX *ctx = wolfSSL_get_SSL_CTX(ssl);
-    struct wolfIP *stack = wolfIP_lookup_stack(ctx);
+    WOLFSSL_CTX *ctx;
+    struct wolfIP *stack;
+
+    if (!ssl)
+        return -1;
+    
+    ctx = wolfSSL_get_SSL_CTX(ssl);
+
+    if (!ctx)
+        return -1;
+
+    stack = wolfIP_lookup_stack(ctx);
+    if (fd < 0)
+        return -1;
 
     if (!stack)
         return WOLFSSL_CBIO_ERR_GENERAL;
@@ -116,6 +137,5 @@ int wolfSSL_SetIO_wolfIP(WOLFSSL* ssl, int fd)
             return 0;
         }
     }
-
     return -1;
 }
diff --git a/src/test/tcp_echo.c b/src/test/tcp_echo.c
@@ -63,6 +63,7 @@ int main() {
     printf("Echo server listening on port %d\n", PORT);
 
     while (1) {
+        ssize_t bytes_read;
         // Accept a client connection
         if ((client_fd = accept(server_fd, (struct sockaddr *)&address, (socklen_t *)&addrlen)) < 0) {
             perror("Accept failed");
@@ -71,7 +72,6 @@ int main() {
 
         printf("Client connected, fd: %d\n", client_fd);
 
-        ssize_t bytes_read;
         while ((bytes_read = read(client_fd, buffer, BUFFER_SIZE)) > 0) {
             write(client_fd, buffer, bytes_read); // Echo data back to the client
         }
diff --git a/src/test/test_ttl_expired.c b/src/test/test_ttl_expired.c
@@ -138,11 +138,14 @@ static struct mem_ep *mem_ep_lookup(struct wolfIP_ll_dev *ll)
 static int mem_ll_poll(struct wolfIP_ll_dev *ll, void *buf, uint32_t len)
 {
     struct mem_ep *ep = mem_ep_lookup(ll);
+    struct mem_link *link;
+    int idx;
+    int ret = 0;
+
     if (!ep)
         return -1;
-    struct mem_link *link = ep->link;
-    int idx = ep->idx;
-    int ret = 0;
+    link = ep->link;
+    idx = ep->idx;
 
     pthread_mutex_lock(&link->lock);
     if (link->ready[idx]) {
@@ -161,10 +164,13 @@ static int mem_ll_poll(struct wolfIP_ll_dev *ll, void *buf, uint32_t len)
 static int mem_ll_send(struct wolfIP_ll_dev *ll, void *buf, uint32_t len)
 {
     struct mem_ep *ep = mem_ep_lookup(ll);
+    struct mem_link *link;
+    int dst;
+
     if (!ep)
         return -1;
-    struct mem_link *link = ep->link;
-    int dst = 1 - ep->idx;
+    link = ep->link;
+    dst = 1 - ep->idx;
 
     pthread_mutex_lock(&link->lock);
     while (link->ready[dst])
@@ -283,8 +289,10 @@ static void *poll_thread(void *arg)
     struct wolfIP *s = (struct wolfIP *)arg;
     while (running) {
         struct timespec ts;
+        uint64_t now;
+
         clock_gettime(CLOCK_MONOTONIC, &ts);
-        uint64_t now = (uint64_t)ts.tv_sec * 1000ULL + ts.tv_nsec / 1000000ULL;
+        now = (uint64_t)ts.tv_sec * 1000ULL + ts.tv_nsec / 1000000ULL;
         wolfIP_poll(s, now);
         usleep(1000);
     }
@@ -293,7 +301,6 @@ static void *poll_thread(void *arg)
 
 int main(void)
 {
-    setvbuf(stdout, NULL, _IONBF, 0);
     struct wolfIP *router;
     struct wolfIP_ll_dev *iface0;
     struct wolfIP_ll_dev *iface1;
@@ -304,6 +311,9 @@ int main(void)
     uint8_t router1_mac[6] = {0x02,0x00,0x00,0x00,0xCC,0x01};
     uint8_t frame[LINK_MTU];
     int rc = EXIT_FAILURE;
+    int n;
+
+    setvbuf(stdout, NULL, _IONBF, 0);
 
     mem_link_init(&link);
     wolfIP_init_static(&router);
@@ -333,7 +343,7 @@ int main(void)
     build_ttl_frame(frame, host_mac, router0_mac, HOST_IP, DEST_IP);
     mem_host_send(&link, frame, sizeof(struct eth_hdr) + sizeof(struct ipv4_hdr) + sizeof(struct icmp_echo));
 
-    int n = mem_host_recv(&link, frame, sizeof(frame), 1000);
+    n = mem_host_recv(&link, frame, sizeof(frame), 1000);
     if (n > 0) {
         struct eth_hdr *eth = (struct eth_hdr *)frame;
         struct ipv4_hdr *ip = (struct ipv4_hdr *)(frame + sizeof(*eth));
diff --git a/src/test/test_wolfssl_forwarding.c b/src/test/test_wolfssl_forwarding.c
diff --git a/src/test/unit/unit.c b/src/test/unit/unit.c
diff --git a/src/wolfip.c b/src/wolfip.c
