From bee1e7088cef913391155f096b42cd4bb89c5c6f Mon Sep 17 00:00:00 2001 From: Tim Date: Sun, 8 Aug 2021 18:24:49 +1000 Subject: [PATCH 1/5] Fix issue #5507 "Constructor WebServer::WebServer(IPAddress addr, int port) produces an unexpected result" "The class Webserver is declared with two explicit constructors, one with signature: WebServer::WebServer(IPAddress addr, int port) Using this results in a server listening on the port number obtained by converting the value of the IPAddress addr argument (in host byte order) to a uint32_t and then to a uint16_t, which is manifestly not the result that would be expected. ... As for a fix, we can assume from these results that this constructor is not being used and therefore could simply be deleted." --- libraries/WebServer/src/WebServer.cpp | 22 ---------------------- libraries/WebServer/src/WebServer.h | 1 - 2 files changed, 23 deletions(-) diff --git a/libraries/WebServer/src/WebServer.cpp b/libraries/WebServer/src/WebServer.cpp index 9f29ff9d2c9..f04ddd17feb 100644 --- a/libraries/WebServer/src/WebServer.cpp +++ b/libraries/WebServer/src/WebServer.cpp @@ -39,28 +39,6 @@ static const char WWW_Authenticate[] = "WWW-Authenticate"; static const char Content_Length[] = "Content-Length"; -WebServer::WebServer(IPAddress addr, int port) -: _corsEnabled(false) -, _server(addr, port) -, _currentMethod(HTTP_ANY) -, _currentVersion(0) -, _currentStatus(HC_NONE) -, _statusChange(0) -, _nullDelay(true) -, _currentHandler(nullptr) -, _firstHandler(nullptr) -, _lastHandler(nullptr) -, _currentArgCount(0) -, _currentArgs(nullptr) -, _postArgsLen(0) -, _postArgs(nullptr) -, _headerKeysCount(0) -, _currentHeaders(nullptr) -, _contentLength(0) -, _chunked(false) -{ -} - WebServer::WebServer(int port) : _corsEnabled(false) , _server(port) diff --git a/libraries/WebServer/src/WebServer.h b/libraries/WebServer/src/WebServer.h index c169da82229..5135700a96f 100644 --- a/libraries/WebServer/src/WebServer.h +++ b/libraries/WebServer/src/WebServer.h @@ -70,7 +70,6 @@ class FS; class WebServer { public: - WebServer(IPAddress addr, int port = 80); WebServer(int port = 80); virtual ~WebServer(); From 4a2034fc334a290ee8ceaeb977e9f7c3b424c506 Mon Sep 17 00:00:00 2001 From: Tim Date: Fri, 13 Aug 2021 06:00:25 +1000 Subject: [PATCH 2/5] Issue 5507 Reverse changes in commit bee1e7088cef913391155f096b42cd4bb89c5c6f after discussion. Alternative fix to be done. --- libraries/WebServer/src/WebServer.cpp | 22 ++++++++++++++++++++++ libraries/WebServer/src/WebServer.h | 1 + 2 files changed, 23 insertions(+) diff --git a/libraries/WebServer/src/WebServer.cpp b/libraries/WebServer/src/WebServer.cpp index f04ddd17feb..9f29ff9d2c9 100644 --- a/libraries/WebServer/src/WebServer.cpp +++ b/libraries/WebServer/src/WebServer.cpp @@ -39,6 +39,28 @@ static const char WWW_Authenticate[] = "WWW-Authenticate"; static const char Content_Length[] = "Content-Length"; +WebServer::WebServer(IPAddress addr, int port) +: _corsEnabled(false) +, _server(addr, port) +, _currentMethod(HTTP_ANY) +, _currentVersion(0) +, _currentStatus(HC_NONE) +, _statusChange(0) +, _nullDelay(true) +, _currentHandler(nullptr) +, _firstHandler(nullptr) +, _lastHandler(nullptr) +, _currentArgCount(0) +, _currentArgs(nullptr) +, _postArgsLen(0) +, _postArgs(nullptr) +, _headerKeysCount(0) +, _currentHeaders(nullptr) +, _contentLength(0) +, _chunked(false) +{ +} + WebServer::WebServer(int port) : _corsEnabled(false) , _server(port) diff --git a/libraries/WebServer/src/WebServer.h b/libraries/WebServer/src/WebServer.h index 5135700a96f..c169da82229 100644 --- a/libraries/WebServer/src/WebServer.h +++ b/libraries/WebServer/src/WebServer.h @@ -70,6 +70,7 @@ class FS; class WebServer { public: + WebServer(IPAddress addr, int port = 80); WebServer(int port = 80); virtual ~WebServer(); From 55a5d039d23205445ee6bafed73e7bbe5d45f91a Mon Sep 17 00:00:00 2001 From: Tim Date: Fri, 13 Aug 2021 06:10:49 +1000 Subject: [PATCH 3/5] Fix issue #5507 "Constructor WebServer::WebServer(IPAddress addr, int port) produces an unexpected result" This change adds support for multi-homed servers to libraries/WiFi. It was assumed to be there already by libraries/WebServer, but was not. This led to unexpected results when the IP address-specific constructor of class WebServer was used (see issue 5507). This change was tested using three concurrent instances of WebServer, one bound to the WiFi station address, one bound to the WiFi soft AP address, and one bound to INADDR_ANY. See libraries/WebServer/examples/MultiHomedServers for the test method. --- .../examples/MultiHomedServers/.gitignore | 1 + .../MultiHomedServers/MultiHomedServers.ino | 174 ++++++++++++++++++ libraries/WebServer/src/WebServer.cpp | 4 +- libraries/WiFi/src/WiFiServer.cpp | 2 +- libraries/WiFi/src/WiFiServer.h | 10 +- 5 files changed, 188 insertions(+), 3 deletions(-) create mode 100644 libraries/WebServer/examples/MultiHomedServers/.gitignore create mode 100644 libraries/WebServer/examples/MultiHomedServers/MultiHomedServers.ino diff --git a/libraries/WebServer/examples/MultiHomedServers/.gitignore b/libraries/WebServer/examples/MultiHomedServers/.gitignore new file mode 100644 index 00000000000..a7fbec47477 --- /dev/null +++ b/libraries/WebServer/examples/MultiHomedServers/.gitignore @@ -0,0 +1 @@ +secret.h diff --git a/libraries/WebServer/examples/MultiHomedServers/MultiHomedServers.ino b/libraries/WebServer/examples/MultiHomedServers/MultiHomedServers.ino new file mode 100644 index 00000000000..9ce570c4862 --- /dev/null +++ b/libraries/WebServer/examples/MultiHomedServers/MultiHomedServers.ino @@ -0,0 +1,174 @@ +#include +#include +#include +#include + +/* + * MultiHomedServers + * + * MultiHomedServers tests support for multi-homed servers, i.e. a distinct web servers on each IP interface. + * It only tests the case n=2 because on a basic ESP32 device, we only have two IP interfaces, namely + * the WiFi station interfaces and the WiFi soft AP interface. + * + * For this to work, the WebServer and the WiFiServer classes must correctly handle the case where an + * IP address is passed to their relevant constructor. It also requires WebServer to work with multiple, + * simultaneous instances. + * + * Testing the WebServer and the WiFiServer constructors was the primary purpose of this script. + * The part of WebServer used by this sketch does seem to work with multiple, simultaneous instances. + * However there is much functionality in WebServer that is not tested here. It may all be well, but + * that is not proven here. + * + * This sketch starts the mDNS server, as did HelloServer, and it resolves esp32.local on both interfaces, + * but was not otherwise tested. + * + * This script also tests that a server not bound to a specific IP address still works. + * + * We create three, simultaneous web servers, one specific to each interface and one that listens on both: + * + * name IP Address Port + * ---- ---------- ---- + * server0 INADDR_ANY 8080 + * server1 station address 8081 + * server2 soft AP address 8082 + * + * The expected responses to a brower's requests are as follows: + * + * 1. when client connected to the same WLAN as the station: + * Request URL Response + * ----------- -------- + * http://stationaddress:8080 "hello from server0" + * http://stationaddress:8081 "hello from server1" + * http://stationaddress:8082 browser reports failure to connect + * + * 2. when client is connected to the soft AP: + * + * Request URL Response + * ----------- -------- + * http://softAPaddress:8080 "hello from server0" + * http://softAPaddress:8082 "hello from server2" + * http://softAPaddress:8081 browser reports failure to connect + * + * 3. Repeat 1 and 2 above with esp32.local in place of stationaddress and softAPaddress, respectively. + * + * MultiHomedServers was originally based on HelloServer. + */ + +#include "secret.h" +const char* ssid = MySSID; +const char* password = MyPASSWORD; +const char *apssid = "ESP32"; + +WebServer *server0, *server1, *server2; + +const int led = 13; + +void handleRoot(WebServer *server, const char *content) { + digitalWrite(led, 1); + server->send(200, "text/plain", content); + digitalWrite(led, 0); +} + +void handleRoot0() { + handleRoot(server0, "hello from server0"); +} + +void handleRoot1() { + handleRoot(server1, "hello from server1"); +} + +void handleRoot2() { + handleRoot(server2, "hello from server2"); +} + +void handleNotFound(WebServer *server) { + digitalWrite(led, 1); + String message = "File Not Found\n\n"; + message += "URI: "; + message += server->uri(); + message += "\nMethod: "; + message += (server->method() == HTTP_GET) ? "GET" : "POST"; + message += "\nArguments: "; + message += server->args(); + message += "\n"; + for (uint8_t i = 0; i < server->args(); i++) { + message += " " + server->argName(i) + ": " + server->arg(i) + "\n"; + } + server->send(404, "text/plain", message); + digitalWrite(led, 0); +} + +void handleNotFound0() { + handleNotFound(server0); +} + +void handleNotFound1() { + handleNotFound(server1); +} + +void handleNotFound2() { + handleNotFound(server2); +} + +void setup(void) { + pinMode(led, OUTPUT); + digitalWrite(led, 0); + Serial.begin(115200); + WiFi.mode(WIFI_STA); + WiFi.begin(ssid, password); + Serial.println(""); + + // Wait for connection + while (WiFi.status() != WL_CONNECTED) { + delay(500); + Serial.print("."); + } + Serial.println(""); + Serial.print("Connected to "); + Serial.println(ssid); + Serial.print("IP address: "); + Serial.println(WiFi.localIP()); + if (!WiFi.softAP(apssid)) { + Serial.println("failed to start softAP"); + for (;;) { + digitalWrite(led, 1); + delay(100); + digitalWrite(led, 0); + delay(100); + } + } + Serial.print("Soft AP: "); + Serial.print(apssid); + Serial.print(" IP address: "); + Serial.println(WiFi.softAPIP()); + + if (MDNS.begin("esp32")) { + Serial.println("MDNS responder started"); + } + + server0 = new WebServer(8080); + server1 = new WebServer(WiFi.localIP(), 8081); + server2 = new WebServer(WiFi.softAPIP(), 8082); + + server0->on("/", handleRoot0); + server1->on("/", handleRoot1); + server2->on("/", handleRoot2); + + server0->onNotFound(handleNotFound0); + server1->onNotFound(handleNotFound1); + server2->onNotFound(handleNotFound2); + + server0->begin(); + Serial.println("HTTP server0 started"); + server1->begin(); + Serial.println("HTTP server1 started"); + server2->begin(); + Serial.println("HTTP server2 started"); +} + +void loop(void) { + server0->handleClient(); + server1->handleClient(); + server2->handleClient(); + delay(2);//allow the cpu to switch to other tasks +} diff --git a/libraries/WebServer/src/WebServer.cpp b/libraries/WebServer/src/WebServer.cpp index 9f29ff9d2c9..dbb3a4fe553 100644 --- a/libraries/WebServer/src/WebServer.cpp +++ b/libraries/WebServer/src/WebServer.cpp @@ -59,6 +59,7 @@ WebServer::WebServer(IPAddress addr, int port) , _contentLength(0) , _chunked(false) { + log_v("WebServer::Webserver(addr=%s, port=%d)", addr.toString().c_str(), port); } WebServer::WebServer(int port) @@ -81,6 +82,7 @@ WebServer::WebServer(int port) , _contentLength(0) , _chunked(false) { + log_v("WebServer::Webserver(port=%d)", port); } WebServer::~WebServer() { @@ -289,7 +291,7 @@ void WebServer::handleClient() { return; } - log_v("New client"); + log_v("New client: client.localIP()=%s", client.localIP().toString().c_str()); _currentClient = client; _currentStatus = HC_WAIT_READ; diff --git a/libraries/WiFi/src/WiFiServer.cpp b/libraries/WiFi/src/WiFiServer.cpp index 7fac7597aa5..db21858125b 100644 --- a/libraries/WiFi/src/WiFiServer.cpp +++ b/libraries/WiFi/src/WiFiServer.cpp @@ -82,7 +82,7 @@ void WiFiServer::begin(uint16_t port, int enable){ return; setsockopt(sockfd, SOL_SOCKET, SO_REUSEADDR, &enable, sizeof(int)); server.sin_family = AF_INET; - server.sin_addr.s_addr = INADDR_ANY; + server.sin_addr.s_addr = _addr; server.sin_port = htons(_port); if(bind(sockfd, (struct sockaddr *)&server, sizeof(server)) < 0) return; diff --git a/libraries/WiFi/src/WiFiServer.h b/libraries/WiFi/src/WiFiServer.h index aeb831db1ba..f5b7eaa7bf3 100644 --- a/libraries/WiFi/src/WiFiServer.h +++ b/libraries/WiFi/src/WiFiServer.h @@ -22,11 +22,14 @@ #include "Arduino.h" #include "Server.h" #include "WiFiClient.h" +#include "arpa/inet.h" +#include "IPAddress.h" class WiFiServer : public Server { private: int sockfd; int _accepted_sockfd = -1; + IPAddress _addr; uint16_t _port; uint8_t _max_clients; bool _listening; @@ -35,7 +38,12 @@ class WiFiServer : public Server { public: void listenOnLocalhost(){} - WiFiServer(uint16_t port=80, uint8_t max_clients=4):sockfd(-1),_accepted_sockfd(-1),_port(port),_max_clients(max_clients),_listening(false),_noDelay(false){} + WiFiServer(uint16_t port=80, uint8_t max_clients=4):sockfd(-1),_accepted_sockfd(-1),_addr(INADDR_ANY),_port(port),_max_clients(max_clients),_listening(false),_noDelay(false) { + log_v("WiFiServer::WiFiServer(port=%d, ...)", port); + } + WiFiServer(const IPAddress& addr, uint16_t port=80, uint8_t max_clients=4):sockfd(-1),_accepted_sockfd(-1),_addr(addr),_port(port),_max_clients(max_clients),_listening(false),_noDelay(false) { + log_v("WiFiServer::WiFiServer(addr=%s, port=%d, ...)", addr.toString().c_str(), port); + } ~WiFiServer(){ end();} WiFiClient available(); WiFiClient accept(){return available();} From 388c6a186ddcf3f670303329b4b351115605f18a Mon Sep 17 00:00:00 2001 From: Tim Date: Fri, 13 Aug 2021 20:04:18 +1000 Subject: [PATCH 4/5] Fix issue #5507 "Constructor WebServer::WebServer(IPAddress addr, int port) produces an unexpected result" (cont.) This fixes what I think might be the cause of CI failures on GitHub for the previous commit, namely the absence of an include file in examples/MultiHomedServers. --- libraries/WebServer/examples/MultiHomedServers/.gitignore | 1 - .../examples/MultiHomedServers/MultiHomedServers.ino | 5 ++--- 2 files changed, 2 insertions(+), 4 deletions(-) delete mode 100644 libraries/WebServer/examples/MultiHomedServers/.gitignore diff --git a/libraries/WebServer/examples/MultiHomedServers/.gitignore b/libraries/WebServer/examples/MultiHomedServers/.gitignore deleted file mode 100644 index a7fbec47477..00000000000 --- a/libraries/WebServer/examples/MultiHomedServers/.gitignore +++ /dev/null @@ -1 +0,0 @@ -secret.h diff --git a/libraries/WebServer/examples/MultiHomedServers/MultiHomedServers.ino b/libraries/WebServer/examples/MultiHomedServers/MultiHomedServers.ino index 9ce570c4862..0e974bfc4e2 100644 --- a/libraries/WebServer/examples/MultiHomedServers/MultiHomedServers.ino +++ b/libraries/WebServer/examples/MultiHomedServers/MultiHomedServers.ino @@ -54,9 +54,8 @@ * MultiHomedServers was originally based on HelloServer. */ -#include "secret.h" -const char* ssid = MySSID; -const char* password = MyPASSWORD; +const char* ssid = "........"; +const char* password = "........"; const char *apssid = "ESP32"; WebServer *server0, *server1, *server2; From 7624736ee748b26aa5669616c862ebc248016932 Mon Sep 17 00:00:00 2001 From: Tim Date: Fri, 13 Aug 2021 21:02:44 +1000 Subject: [PATCH 5/5] Fix issue #5507 "Constructor WebServer::WebServer(IPAddress addr, int port) produces an unexpected result" (cont.) Change port numbers in examples/MultiHomedServers per pull-request comment from me-no-dev ... "for this test to be valid, both servers should be on the same port. That is how you can make sure that the functionality works." --- .../examples/MultiHomedServers/MultiHomedServers.ino | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/libraries/WebServer/examples/MultiHomedServers/MultiHomedServers.ino b/libraries/WebServer/examples/MultiHomedServers/MultiHomedServers.ino index 0e974bfc4e2..0e77fc3dd64 100644 --- a/libraries/WebServer/examples/MultiHomedServers/MultiHomedServers.ino +++ b/libraries/WebServer/examples/MultiHomedServers/MultiHomedServers.ino @@ -30,7 +30,7 @@ * ---- ---------- ---- * server0 INADDR_ANY 8080 * server1 station address 8081 - * server2 soft AP address 8082 + * server2 soft AP address 8081 * * The expected responses to a brower's requests are as follows: * @@ -39,15 +39,13 @@ * ----------- -------- * http://stationaddress:8080 "hello from server0" * http://stationaddress:8081 "hello from server1" - * http://stationaddress:8082 browser reports failure to connect * * 2. when client is connected to the soft AP: * * Request URL Response * ----------- -------- * http://softAPaddress:8080 "hello from server0" - * http://softAPaddress:8082 "hello from server2" - * http://softAPaddress:8081 browser reports failure to connect + * http://softAPaddress:8081 "hello from server2" * * 3. Repeat 1 and 2 above with esp32.local in place of stationaddress and softAPaddress, respectively. * @@ -147,7 +145,7 @@ void setup(void) { server0 = new WebServer(8080); server1 = new WebServer(WiFi.localIP(), 8081); - server2 = new WebServer(WiFi.softAPIP(), 8082); + server2 = new WebServer(WiFi.softAPIP(), 8081); server0->on("/", handleRoot0); server1->on("/", handleRoot1);