From befc74e0f0dbe34ecd8059fef5d4f73a0b277f41 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 dc4cbd86e..86805c7d9 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; @@ -335,6 +340,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 a5bd08a6b65362c21c137bb9e457a3956f6e8d17 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 e343c99ca..d7a8542aa 100644 --- a/components/esp_http_server/test/test_http_server.c +++ b/components/esp_http_server/test/test_http_server.c @@ -167,3 +167,22 @@ TEST_CASE("Basic Functionality Tests", "[HTTP SERVER]") test_handler_limit(hd); TEST_ASSERT(httpd_stop(hd) == ESP_OK); } + +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); +}