Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Constructor WebServer::WebServer(IPAddress addr, int port) produces an unexpected result #5507

Closed
timr49 opened this issue Aug 8, 2021 · 1 comment

Comments

@timr49
Copy link
Contributor

timr49 commented Aug 8, 2021

Hardware:

Board: Sparkfun ESP32Thing
Core Installation version: 2.0.0-rc1
IDE name: Arduino IDE 1.8.5
Flash Frequency: 80Mhz
PSRAM enabled: ?PSRAM?
Upload Speed: 921600
Computer OS: Mac OSX 11.5.1

Description:

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.

The reason for this is that the definition of this constructor calls the WiFiServer constructor as _server(addr, port).
The class WiFiServer has exactly one explicit constructor with signature:
WiFiServer(uint16_t port=80, uint8_t max_clients=4).
This does not result in a compile-time error because class IPAddress declares an "operator uint32_t" and hence addr is implicitly cast to uint32_t from which it is implicitly case to uint16_t as required by WiFiserver constructor.

This can be demonstrated with the sketch below. A browser was used to make the HTTP request to the URL:
http://192.168.10.157:43200/
where the port number 43200 was chosen from the number displayed by the line:
Serial.print("(uint16_t)WiFi.localIP()="); Serial.println((uint16_t)WiFi.localIP(), DEC);
in the sketch, namely:
(uint16_t)WiFi.localIP()=43200

The Core Debug Level was set to Verbose so that the port number actually used in the HTTP request is exposed by the line:
log_v("headerValue: %s", headerValue.c_str());
in Parsing.cpp, namely:
[395236][V][Parsing.cpp:228] _parseRequest(): headerValue: 192.168.10.157:43200

In this case the port number is 43200 because the IP address is 192.168.10.157 but in general it will vary depending on the server's IP address.

As for a fix, we can assume from these results that this constructor is not being used and therefore could simply be deleted.

Sketch: (leave the backquotes for code formatting)

//Change the code below by your sketch
[code]
#include <WiFi.h>
#include <WebServer.h>
#include "secret.h" // for definitions of MySSID and MyPASSWORD

WebServer *server;

void setup() {
  // put your setup code here, to run once:
  // serial
  Serial.begin(115200);
  while (!Serial)
    delay(100);
  Serial.println("setting up");

  // wifi
  WiFi.begin(MySSID, MyPASSWORD); // as defined in "secret.h"
  while (!WiFi.isConnected())
  {
    Serial.print('.');
    delay(100);
  }
  Serial.println();
  Serial.print("connected to "); Serial.print(WiFi.SSID());
  Serial.print(" with address "); Serial.println(WiFi.localIP());

  // www server
  Serial.print("(uint32_t)WiFi.localIP()="); Serial.println((uint32_t)WiFi.localIP(), DEC);
  Serial.print("(uint16_t)WiFi.localIP()="); Serial.println((uint16_t)WiFi.localIP(), DEC);
  server = new WebServer(WiFi.localIP(), 80); // using constructor with signature WebServer(IPAddress addr, int port = 80)
  if (server)
  {
    server->on("/", []() { server->send(200, "text/html", "<!DOCTYPE html><html><body>hello world</body></html>"); });
    server->begin();
    Serial.println("listening, but on which port?");
  }
  else
  {
    Serial.println("new WebServer failed");
  }
}

void loop() {
  // put your main code here, to run repeatedly:
  if (server)
    server->handleClient();
}
[/code]```

### Debug Messages:

ets Jun 8 2016 00:22:57

rst:0x10 (RTCWDT_RTC_RESET),boot:0x13 (SPI_FAST_FLASH_BOOT)
configsip: 0, SPIWP:0xee
clk_drv:0x00,q_drv:0x00,d_drv:0x00,cs0_drv:0x00,hd_drv:0x00,wp_drv:0x00
mode:DIO, clock div:1
load:0x3fff0030,len:1100
ho 0 tail 12 room 4
load:0x40078000,len:12752
load:0x40080400,len:3092
entry 0x400805e4
[�� $HH⸮LW+QW⸮Y.&⸮KZX⸮K,]⸮,'⸮LLW⸮⸮Y](]⸮⸮equencyMhz(): PLL: 480 / 2 = 240 Mhz, APB: 80000000 Hz
⸮setting up
[ 63][D][WiFiGeneric.cpp:808] _eventCallback(): Arduino Event: 0 - WIFI_READY
[ 159][V][WiFiGeneric.cpp:96] set_esp_interface_ip(): Configuring Station static IP: 0.0.0.0, MASK: 0.0.0.0, GW: 0.0.0.0
[ 158][V][WiFiGeneric.cpp:272] _arduino_event_cb(): STA Started
[ 165][D][WiFiGeneric.cpp:808] _eventCallback(): Arduino Event: 2 - STA_START
.....................[ 2276][V][WiFiGeneric.cpp:284] _arduino_event_cb(): STA Connected: SSID: MOBILE3, BSSID: 00:1e:42:19:71:d9, Channel: 11, Auth: WPA2_PSK
[ 2277][D][WiFiGeneric.cpp:808] _eventCallback(): Arduino Event: 4 - STA_CONNECTED
.[ 2327][V][WiFiGeneric.cpp:294] _arduino_event_cb(): STA Got New IP:192.168.10.157
[ 2328][D][WiFiGeneric.cpp:808] _eventCallback(): Arduino Event: 7 - STA_GOT_IP
[ 2331][D][WiFiGeneric.cpp:857] _eventCallback(): STA IP: 192.168.10.157, MASK: 255.255.255.0, GW: 192.168.10.3

connected to MOBILE3 with address 192.168.10.157
(uint32_t)WiFi.localIP()=2634721472
(uint16_t)WiFi.localIP()=43200
listening, but on which port?
[384334][V][WebServer.cpp:292] handleClient(): New client
[389338][V][WebServer.cpp:292] handleClient(): New client
[394342][V][WebServer.cpp:292] handleClient(): New client
[394343][V][Parsing.cpp:121] _parseRequest(): method: GET url: / search:
[394343][V][Parsing.cpp:227] _parseRequest(): headerName: Host
[394348][V][Parsing.cpp:228] _parseRequest(): headerValue: 192.168.10.157:43200
[394355][V][Parsing.cpp:227] _parseRequest(): headerName: Upgrade-Insecure-Requests
[394362][V][Parsing.cpp:228] _parseRequest(): headerValue: 1
[394368][V][Parsing.cpp:227] _parseRequest(): headerName: Accept
[394374][V][Parsing.cpp:228] _parseRequest(): headerValue: text/html,application/xhtml+xml,application/xml;q=0.9,/;q=0.8
[394385][V][Parsing.cpp:227] _parseRequest(): headerName: User-Agent
[394390][V][Parsing.cpp:228] _parseRequest(): headerValue: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/14.1.2 Safari/605.1.15
[394406][V][Parsing.cpp:227] _parseRequest(): headerName: Accept-Language
[394413][V][Parsing.cpp:228] _parseRequest(): headerValue: en-au
[394418][V][Parsing.cpp:227] _parseRequest(): headerName: Accept-Encoding
[394425][V][Parsing.cpp:228] _parseRequest(): headerValue: gzip, deflate
[394431][V][Parsing.cpp:227] _parseRequest(): headerName: Connection
[394437][V][Parsing.cpp:228] _parseRequest(): headerValue: keep-alive
[394443][V][Parsing.cpp:255] _parseArguments(): args:
[394448][V][Parsing.cpp:238] _parseRequest(): Request: /
[394453][V][Parsing.cpp:239] _parseRequest(): Arguments:
[395189][V][WebServer.cpp:292] handleClient(): New client
[395234][V][Parsing.cpp:121] _parseRequest(): method: GET url: /favicon.ico search:
[395235][V][Parsing.cpp:227] _parseRequest(): headerName: Host
[395236][V][Parsing.cpp:228] _parseRequest(): headerValue: 192.168.10.157:43200
[395244][V][Parsing.cpp:227] _parseRequest(): headerName: Connection
[395250][V][Parsing.cpp:228] _parseRequest(): headerValue: keep-alive
[395256][V][Parsing.cpp:227] _parseRequest(): headerName: Accept
[395261][V][Parsing.cpp:228] _parseRequest(): headerValue: /
[395267][V][Parsing.cpp:227] _parseRequest(): headerName: User-Agent
[395273][V][Parsing.cpp:228] _parseRequest(): headerValue: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/14.1.2 Safari/605.1.15
[395289][V][Parsing.cpp:227] _parseRequest(): headerName: Accept-Language
[395295][V][Parsing.cpp:228] _parseRequest(): headerValue: en-au
[395301][V][Parsing.cpp:227] _parseRequest(): headerName: Referer
[395307][V][Parsing.cpp:228] _parseRequest(): headerValue: http://192.168.10.157:43200/
[395315][V][Parsing.cpp:227] _parseRequest(): headerName: Accept-Encoding
[395321][V][Parsing.cpp:228] _parseRequest(): headerValue: gzip, deflate
[395327][V][Parsing.cpp:255] _parseArguments(): args:
[395332][V][Parsing.cpp:238] _parseRequest(): Request: /favicon.ico
[395338][V][Parsing.cpp:239] _parseRequest(): Arguments:
[395344][E][WebServer.cpp:633] _handleRequest(): request handler not found

timr49 added a commit to timr49/arduino-esp32 that referenced this issue Aug 8, 2021
…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."
timr49 added a commit to timr49/arduino-esp32 that referenced this issue Aug 12, 2021
…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.
timr49 added a commit to timr49/arduino-esp32 that referenced this issue Aug 13, 2021
…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.
timr49 added a commit to timr49/arduino-esp32 that referenced this issue Aug 13, 2021
…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."
me-no-dev pushed a commit that referenced this issue Aug 23, 2021
#5509)

* 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."

* Issue 5507
Reverse changes in commit bee1e70 after discussion.
Alternative fix to be done.

* 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.

* 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.

* 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."
@VojtechBartoska
Copy link
Contributor

Thanks for your contribution! Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants