Skip to content

WebServer::args() ignores empty parameters #6759

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

Closed
1 task done
tsctrl opened this issue May 16, 2022 · 19 comments
Closed
1 task done

WebServer::args() ignores empty parameters #6759

tsctrl opened this issue May 16, 2022 · 19 comments
Labels
Area: BT&Wifi BT & Wifi related issues Type: Feature request Feature request for Arduino ESP32

Comments

@tsctrl
Copy link

tsctrl commented May 16, 2022

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

  • I confirm I have checked existing issues, online documentation and Troubleshooting guide.
@tsctrl tsctrl added the Status: Awaiting triage Issue is waiting for triage label May 16, 2022
@VojtechBartoska VojtechBartoska added the Area: BT&Wifi BT & Wifi related issues label May 17, 2022
@SuGlider SuGlider added Status: Test needed Issue needs testing Status: Needs investigation We need to do some research before taking next steps on this issue labels May 17, 2022
@SuGlider
Copy link
Collaborator

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

@PilnyTomas
Copy link
Contributor

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

@tsctrl
Copy link
Author

tsctrl commented May 21, 2022

edited as per request and will compile

@PilnyTomas
Copy link
Contributor

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
Copy link
Author

tsctrl commented May 24, 2022

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

@PilnyTomas
Copy link
Contributor

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
Copy link
Collaborator

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
Copy link
Contributor

@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
Copy link
Collaborator

The URL may have a Query string which starts after "?" symbol as this general format:
scheme://username:[email protected]: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:

  • k1 = ""
  • k2 = v2
  • k3 = ""
  • k4 = ""
  • k5 = v5

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

@PilnyTomas PilnyTomas changed the title WebServer query params return wrong args count WebServer::args() ignores empty parameters May 26, 2022
@tsctrl
Copy link
Author

tsctrl commented May 26, 2022

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
Copy link
Contributor

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 VojtechBartoska added Type: Feature request Feature request for Arduino ESP32 and removed Status: Test needed Issue needs testing Status: Awaiting triage Issue is waiting for triage labels May 27, 2022
@VojtechBartoska
Copy link
Contributor

Adding feature request label, we will evaluate this soon.

@VojtechBartoska
Copy link
Contributor

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

@SuGlider SuGlider self-assigned this Jun 15, 2022
@VojtechBartoska VojtechBartoska removed this from the 2.0.4 milestone Jun 15, 2022
@VojtechBartoska
Copy link
Contributor

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

@VojtechBartoska VojtechBartoska added this to the 2.1.0 milestone Jun 15, 2022
@VojtechBartoska VojtechBartoska modified the milestones: 2.0.6, 3.0.0 Sep 21, 2022
@PepeTheFroggie
Copy link

PepeTheFroggie commented Jan 11, 2023

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
Copy link

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
Copy link

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

@unimo-chan
Copy link

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;
}

@VojtechBartoska VojtechBartoska modified the milestones: 3.0.0, 3.1.0 Feb 20, 2024
@rftafas rftafas modified the milestones: 3.1.0, 3.1.1 Jan 6, 2025
@rftafas rftafas removed this from the 3.1.1 milestone Jan 14, 2025
@Parsaabasi Parsaabasi removed the Status: Needs investigation We need to do some research before taking next steps on this issue label Jan 16, 2025
@Parsaabasi
Copy link

Hello,

Due to the overwhelming volume of issues currently being addressed, we have decided to close the previously received tickets. If you still require assistance or if the issue persists, please don't hesitate to reopen the ticket.

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: BT&Wifi BT & Wifi related issues Type: Feature request Feature request for Arduino ESP32
Projects
Development

No branches or pull requests

9 participants