From e90c90d1f6143cee3198c9002085472d3647dee5 Mon Sep 17 00:00:00 2001 From: Anurag Kar Date: Mon, 1 Apr 2019 14:52:04 +0530 Subject: [PATCH 1/2] esp_http_server : Only accept new connections if server has capacity to handle more This fix prevents HTTP server from accepting new connections when the total count of connected sockets has reached the max_open_sockets limit set during configuration. The pending connections are kept in backlog until atleast one of the connected sockets is closed. The maximum number of connection requests that can kept in backlog is specified as backlog_conn configuration option. Note that this modification has no effect when LRU purge is enabled. Also added sanity check on setting for max_open_sockets during configuration. Solution suggested by jimparis https://github.com/espressif/esp-idf/issues/3183#issue-421234265 Closes https://github.com/espressif/esp-idf/issues/3183 --- components/esp_http_server/src/httpd_main.c | 24 ++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/components/esp_http_server/src/httpd_main.c b/components/esp_http_server/src/httpd_main.c index 52228a97c..d361bfee6 100644 --- a/components/esp_http_server/src/httpd_main.c +++ b/components/esp_http_server/src/httpd_main.c @@ -156,7 +156,12 @@ static esp_err_t httpd_server(struct httpd_data *hd) { fd_set read_set; FD_ZERO(&read_set); - FD_SET(hd->listen_fd, &read_set); + if (hd->config.lru_purge_enable || httpd_is_sess_available(hd)) { + /* Only listen for new connections if server has capacity to + * handle more (or when LRU purge is enabled, in which case + * older connections will be closed) */ + FD_SET(hd->listen_fd, &read_set); + } FD_SET(hd->ctrl_fd, &read_set); int tmp_max_fd; @@ -348,6 +353,23 @@ esp_err_t httpd_start(httpd_handle_t *handle, const httpd_config_t *config) return ESP_ERR_INVALID_ARG; } + /* Sanity check about whether LWIP is configured for providing the + * maximum number of open sockets sufficient for the server. Though, + * this check doesn't guarantee that many sockets will actually be + * available at runtime as other processes may use up some sockets. + * Note that server also uses 3 sockets for its internal use : + * 1) listening for new TCP connections + * 2) for sending control messages over UDP + * 3) for receiving control messages over UDP + * So the total number of required sockets is max_open_sockets + 3 + */ + if (CONFIG_LWIP_MAX_SOCKETS < config->max_open_sockets + 3) { + ESP_LOGE(TAG, "Configuration option max_open_sockets is too large (max allowed %d)\n\t" + "Either decrease this or configure LWIP_MAX_SOCKETS to a larger value", + CONFIG_LWIP_MAX_SOCKETS - 3); + return ESP_ERR_INVALID_ARG; + } + struct httpd_data *hd = httpd_create(config); if (hd == NULL) { /* Failed to allocate memory */ From 8bd09fb0a52df5bc477977d530087c464e7310a2 Mon Sep 17 00:00:00 2001 From: Anurag Kar Date: Wed, 3 Apr 2019 19:01:40 +0530 Subject: [PATCH 2/2] esp_http_server : Test added to check limit on max_open_sockets config option --- .../esp_http_server/test/test_http_server.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/components/esp_http_server/test/test_http_server.c b/components/esp_http_server/test/test_http_server.c index 682a54d03..186df6c9b 100644 --- a/components/esp_http_server/test/test_http_server.c +++ b/components/esp_http_server/test/test_http_server.c @@ -232,3 +232,22 @@ TEST_CASE("URI Wildcard Matcher Tests", "[HTTP SERVER]") ut++; } } + +TEST_CASE("Max Allowed Sockets Test", "[HTTP SERVER]") +{ + test_case_uses_tcpip(); + + httpd_handle_t hd; + httpd_config_t config = HTTPD_DEFAULT_CONFIG(); + + /* Starting server with default config options should pass */ + TEST_ASSERT(httpd_start(&hd, &config) == ESP_OK); + TEST_ASSERT(httpd_stop(hd) == ESP_OK); + + /* Default value of max_open_sockets is already set as per + * maximum limit imposed by LWIP. Increasing this beyond the + * maximum allowed value, without increasing LWIP limit, + * should fail */ + config.max_open_sockets += 1; + TEST_ASSERT(httpd_start(&hd, &config) != ESP_OK); +}