espressif / arduino-esp32

Arduino core for the ESP32
GNU Lesser General Public License v2.1
13k stars 7.29k forks source link

WebServer::args() ignores empty parameters #6759

Open tsctrl opened 2 years ago

tsctrl commented 2 years ago

Board

ESP32 Dev Module

Device Description

DevKitC

Hardware Configuration

-

Version

v2.0.3

IDE Name

IDF Component, Arduino IDE

Operating System

Windows 10

Flash frequency

40MHz

PSRAM enabled

no

Upload speed

115200

Description

WebServer args() return wrong count if params is without equal= value

to reproduce use uri/?query as url to handle

refer example sketch as below. is this is valid result? opposite with ESPAsyncWebServer libs request.params() count without equal= after ? as +1

Sketch


/*
    This sketch shows how to configure different external or internal clock sources for the Ethernet PHY
*/

#include <ETH.h>
#include <WiFi.h>
#include <WebServer.h>

/* 
   * ETH_CLOCK_GPIO0_IN   - default: external clock from crystal oscillator
   * ETH_CLOCK_GPIO0_OUT  - 50MHz clock from internal APLL output on GPIO0 - possibly an LAN8720 is needed for LAN8720
   * ETH_CLOCK_GPIO16_OUT - 50MHz clock from internal APLL output on GPIO16 - possibly an inverter is needed for LAN8720
   * ETH_CLOCK_GPIO17_OUT - 50MHz clock from internal APLL inverted output on GPIO17 - tested with LAN8720
*/
#define ETH_CLK_MODE    ETH_CLOCK_GPIO17_OUT

// Pin# of the enable signal for the external crystal oscillator (-1 to disable for internal APLL source)
#define ETH_POWER_PIN   -1

// Type of the Ethernet PHY (LAN8720 or TLK110)
#define ETH_TYPE        ETH_PHY_LAN8720

// I²C-address of Ethernet PHY (0 or 1 for LAN8720, 31 for TLK110)
#define ETH_ADDR        1

// Pin# of the I²C clock signal for the Ethernet PHY
#define ETH_MDC_PIN     23

// Pin# of the I²C IO signal for the Ethernet PHY
#define ETH_MDIO_PIN    18

WebServer server(80); 
static bool eth_connected = false;

void handle(){
  Serial.println(server.args());
}

void WiFiEvent(WiFiEvent_t event) {
  switch (event) {
    case SYSTEM_EVENT_ETH_START:
      Serial.println("ETH Started");
      //set eth hostname here
      ETH.setHostname("esp32-ethernet");
      break;
    case SYSTEM_EVENT_ETH_CONNECTED:
      Serial.println("ETH Connected");
      break;
    case SYSTEM_EVENT_ETH_GOT_IP:
      Serial.print("ETH MAC: ");
      Serial.print(ETH.macAddress());
      Serial.print(", IPv4: ");
      Serial.print(ETH.localIP());
      if (ETH.fullDuplex()) {
        Serial.print(", FULL_DUPLEX");
      }
      Serial.print(", ");
      Serial.print(ETH.linkSpeed());
      Serial.println("Mbps");
      eth_connected = true;
      break;
    case SYSTEM_EVENT_ETH_DISCONNECTED:
      Serial.println("ETH Disconnected");
      eth_connected = false;
      break;
    case SYSTEM_EVENT_ETH_STOP:
      Serial.println("ETH Stopped");
      eth_connected = false;
      break;
    default:
      break;
  }
}

void testClient(const char * host, uint16_t port) {
  Serial.print("\nconnecting to ");
  Serial.println(host);

  WiFiClient client;
  if (!client.connect(host, port)) {
    Serial.println("connection failed");
    return;
  }
  client.printf("GET / HTTP/1.1\r\nHost: %s\r\n\r\n", host);
  while (client.connected() && !client.available());
  while (client.available()) {
    Serial.write(client.read());
  }

  Serial.println("closing connection\n");
  client.stop();
}

void setup() {
  Serial.begin(115200);
  WiFi.onEvent(WiFiEvent);
  ETH.begin(ETH_ADDR, ETH_POWER_PIN, ETH_MDC_PIN, ETH_MDIO_PIN, ETH_TYPE, ETH_CLK_MODE);

  delay(1000); //coffee break;
  server.on("", HTTP_GET, handle);
  server.on("/", HTTP_GET, handle);
  server.begin();

}

void loop() {
  if (eth_connected) {
    testClient("google.com", 80);
    server.handleClient();
  }
  delay(1);
}

Debug Message

as above sketch

in handle:
url:http://192.168.4.1/?getnetwork
server.args() = 0 (wrong count)

url:http://192.168.4.1/?getnetwork=1&debug=true
server.args() = 2 (correct)

url:http://192.168.4.1/?getnetwork&debug=true
server.args() = 1 (wrong count)

Other Steps to Reproduce

No response

I have checked existing issues, online documentation and the Troubleshooting Guide

SuGlider commented 2 years ago

@PilnyTomas - Would you like to test it and investigate this issue? Let me know.

PilnyTomas commented 2 years ago

Hi @tsctrl, please modify the sketch to be complete - i.e. when I copy-paste it into Arduino IDE it will compile.

tsctrl commented 2 years ago

edited as per request and will compile

PilnyTomas commented 2 years ago

A web server needs some connection, there is no web connection in the provided sketch, and it crashes on startup. Please provide a complete and minimal sketch that demonstrates the issue. A modified example from the library can be used too.

Complete = compiles and starts properly after copy-paste (only change needed in code relates to SSID+PWD) Minimal = only code needed to demonstrate the issue - issue demonstration does not require the rest of the system for which it is used.

tsctrl commented 2 years ago

updated with a complete and minimal sketch. does not require ssid and password

PilnyTomas commented 2 years ago

Can you please explain how did you manage to get working ETHernet on a bare ESP32 DevKitC? I also don't understand why you mix WiFi and Ethernet.

Please provide a complete and minimal sketch that demonstrates the issue. A modified example from the library can be used too.

Complete = compiles and starts properly after copy-paste (only change needed in code relates to SSID+PWD) Minimal = only code needed to demonstrate the issue - issue demonstration does not require the rest of the system for which it is used.

Please provide an example that is able to reproduce your issue with all other needed steps to reproduce.

SuGlider commented 2 years ago

The real question of this issue is:

refer example sketch as below. is this is valid result? opposite with
ESPAsyncWebServer libs request.params() count without equal= after ? as +1

example with WebServer:

url:http://192.168.4.1/?getnetwork
server.args() = 0 (wrong count)

url:http://192.168.4.1/?getnetwork=1&debug=true
server.args() = 2 (correct)

url:http://192.168.4.1/?getnetwork&debug=true
server.args() = 1 (wrong count)

This related to WebServer Library in Parsing.cpp -- it doesn't matter if it is using ETH or WiFi.

PilnyTomas commented 2 years ago

@SuGlider, I created my own code and if the argument has an equal sign it works ok every time. I think this problem is due to poor documentation - users don't know what to expect.

SuGlider commented 2 years ago

The URL may have a Query string which starts after "?" symbol as this general format: scheme://username:password@subdomain.domain.tld:port/path/file-name.suffix?query-string#hash

The syntax of the Query is not specified in the RFC 3986. But it is recommended (not mandatory) that Query is formed by pairs of key=value separated by "&".

The question here is if the WebServer class shall consider a Query expression with no value or with no "=" symbol as valid and meaningful. It has to do with parsing of the URL and what the server side considers as valid or not.

For instance: http://192.168.1.1/page?k1&k2=v2&k3=&k4&k5=v5 is a valid URL. Query string is k1&k2=v2&k3=&k4&k5=v5 and should split into these 5 arguments:

So, I think that we have an issue here, as presented by @tsctrl

tsctrl commented 2 years ago

thanks @PilnyTomas , @SuGlider

webserver prerequisite a value for a key to count it as a valid args. is not an issue, but it is convenient to have a key without value as it is meaningful to execute function by just validating the key itself and client did not have to hard follow the key value format.

personally, url?query&example looks and sound better than adding url?query=""&example="" in the browser address bar.

PilnyTomas commented 2 years ago

Hi, @tsctrl I agree. We might add support for an empty parameter in the future, but I don't want to promise anything right now. We will discuss it internally...

VojtechBartoska commented 2 years ago

Adding feature request label, we will evaluate this soon.

VojtechBartoska commented 2 years ago

Other option to be consider is to refactor whole WebServer to use ESP-IDF solution.

VojtechBartoska commented 2 years ago

Removing this issue from 2.0.4 and adding it under 2.1.0 milestone as WebServer needs more investigation.

PepeTheFroggie commented 1 year ago

Same issue here. No = means no arg. On the esp8266 this works perfectly. When transfering esp8266 code to esp32 this needs 2 hours debugging :(

example works 8266, doesent work esp32: out += "<button onclick=\"window.location.href='/cmd?rd\';\">Read\n";

esp32 needs: out += "<button onclick=\"window.location.href='/cmd?rd=1\';\">Read\n";

cor-of-org commented 1 year ago

Ow! I just spent the last half hour trying to figure out wtf was going on here. I want to send a simple /url?SomeCommand, which is parsed elsehwere (and used by serial connexion). I figured out I need to send command=value and then conctenate them back together for parsing! lol. Can we please get this working!

PepeTheFroggie commented 10 months ago

This has still not been resolved. Need help? Ran into the same problem converting esp8266 code to esp32-c3 !

unimo-chan commented 9 months ago

Hi, I could probably fix it with the code below. I use 1.0.6.

See WebServer::_parseArguments(String data) (in Parsing.cpp)

rewrite

if ((equal_sign_index == -1) || ((equal_sign_index > next_arg_index) && (next_arg_index != -1))) { log_e("arg missing value: %d", iarg); if (next_arg_index == -1) break; pos = next_arg_index + 1; continue; } RequestArgument& arg = _currentArgs[iarg];

to

RequestArgument& arg = _currentArgs[iarg];

if ((equal_sign_index == -1) || ((equal_sign_index > next_arg_index) && (next_arg_index != -1))) {
  log_e("arg missing value: %d", iarg);
  arg.value = "";
  ++iarg;
  if (next_arg_index == -1){
    arg.key = urlDecode(data.substring(pos, (int)data.length()));
    break;
  }
  arg.key = urlDecode(data.substring(pos,  next_arg_index));
  pos = next_arg_index + 1;
  continue;
}