WiFiUDP.remoteIP returns incorrect value in 2.5.0

[cjdshaw]

Basic Infos

• This issue complies with the issue POLICY doc.
• I have read the documentation at readthedocs and the issue is not addressed there.
• I have tested that the issue is present in current master branch (aka latest git).
• I have searched the issue tracker for a similar issue.
• If there is a stack dump, I have decoded it.
• I have filled out all fields below.

Platform

• Hardware: [ESP-12]
• Core Version: [2.5.0]
• Development Env: [Arduino IDE]
• Operating System: [MacOS]

Settings in IDE

• Module: [Wemos D1 mini r2]
• Flash Mode: [qio]
• Flash Size: [4MB/1MB]
• lwip Variant: [v2 Lower Memory]
• Reset Method: [ck]
• Flash Frequency: [40Mhz]
• CPU Frequency: [80Mhz]
• Upload Using: [SERIAL]
• Upload Speed: [921600] (serial upload only)

Problem Description

When receiving UDP packets from multiple sources, WiFiUDP.remoteIP only updates the first few times. After that, it returns the same, incorrect value.

My sketch broadcasts a request for information to my network. Multiple smart plugs (TP-Link HS100s) respond. The sketch takes each response in turn and gets its source IP.

With board version 2.4.2, I get a unique IP for each response, which correspond to the names in the packet (decryption and parsing not shown in sketch)

2.4.2:
Connecting to wifi..
WiFi connected. IP = 192.168.180.56
Received 574 bytes from 192.168.180.33, port 9999
Received 574 bytes from 192.168.180.47, port 9999
Received 673 bytes from 192.168.180.39, port 9999
Received 571 bytes from 192.168.180.55, port 9999
Received 568 bytes from 192.168.180.54, port 9999
Received 566 bytes from 192.168.180.38, port 9999
Received 571 bytes from 192.168.180.3, port 9999

With board version 2.4.2, I get the same number of responses, but multiple repeated IPs which do not correspond to the names in the packets (decryption and parsing not shown in sketch)

2.5.0:
Connecting to wifi..
WiFi connected. IP = 192.168.180.56
Received 574 bytes from 192.168.180.33, port 9999
Received 574 bytes from 192.168.180.47, port 9999
Received 571 bytes from 192.168.180.55, port 9999
Received 568 bytes from 192.168.180.3, port 9999
Received 566 bytes from 192.168.180.3, port 9999
Received 673 bytes from 192.168.180.3, port 9999
Received 571 bytes from 192.168.180.3, port 9999

MCVE Sketch

#include <ESP8266WiFi.h>
#include <WiFiUdp.h>

void TPL_encrypt( char *msg ) {
  char *cptr, key = 171;

  cptr = (char *)msg;
  while ( *cptr ) {
    //    Serial.print((char )*cptr);
    //    Serial.print(" > ");
    *cptr = *cptr ^ key;
    //    Serial.println((unsigned int )*cptr);
    key = *cptr;
    cptr++;
  }
}

void TPLsend( WiFiUDP *Udp, String cmd ) {
  IPAddress TPLIP = WiFi.localIP();
  TPLIP[3] = 255;

  Udp->beginPacket(TPLIP, 9999);
  int l = cmd.length();
  char buf[1024];
  cmd.toCharArray( buf, 1024 );
  TPL_encrypt( buf );
  Udp->write(buf, l);
  Udp->endPacket();
}

void setup() {

  Serial.begin(115200);
  delay(10);

  Serial.print("Connecting to wifi");
  while ( WiFi.status() != WL_CONNECTED ) {
    delay(1000);
    Serial.print(".");
  }
  Serial.println("");
  Serial.print("WiFi connected. IP = ");
  Serial.println(WiFi.localIP().toString());


  WiFiUDP Udp;
  Udp.begin(9999);

  TPLsend( &Udp, "{\"system\":{\"get_sysinfo\":{}}}" );

  char incomingPacket[1024];  // buffer for incoming packets
  while ( 1 ) {
    int packetSize = Udp.parsePacket();
    if (packetSize) {
      Serial.printf("Received %d bytes from %s, port %d\n", packetSize, Udp.remoteIP().toString().c_str(), Udp.remotePort());
    }
    delay( 10 );
  }
  Udp.stop();
}

void loop() {

}

Debug Messages

SDK:3.0.0-dev(c0f7b44)/Core:2.5.0=20500000/lwIP:STABLE-2_1_2_RELEASE/glue:1.1/BearSSL:6778687
wifi evt: 2
Connecting to wifiscandone
state: 0 -> 2 (b0)
state: 2 -> 3 (0)
state: 3 -> 5 (10)
add 0
aid 14
cnt 

connected with xxxxxxxx, channel 8
dhcp client start...
wifi evt: 0
.....ip:192.168.180.56,mask:255.255.255.0,gw:192.168.180.1
wifi evt: 3
.
WiFi connected. IP = 192.168.180.56
:urn 574
:urch 574, 574
Received 574 bytes from 192.168.180.47, port 9999
Received 574 bytes from 192.168.180.47, port 9999
:urn 673
Received 673 bytes from 192.168.180.39, port 9999
:urch 673, 571
:urch 1244, 571
:urch 1815, 568
:urch 2383, 566
Received 571 bytes from 192.168.180.38, port 9999
Received 571 bytes from 192.168.180.38, port 9999
Received 568 bytes from 192.168.180.38, port 9999
Received 566 bytes from 192.168.180.38, port 9999
pm open,type:2 0

[d-a-v]

:urch 673, 571
:urch 1244, 571
:urch 1815, 568
:urch 2383, 566
Received 571 bytes from 192.168.180.38, port 9999
Received 571 bytes from 192.168.180.38, port 9999
Received 568 bytes from 192.168.180.38, port 9999
Received 566 bytes from 192.168.180.38, port 9999

This log explains what happens. UDP remote address is (now-2.5.0) overwritten on each new received packet.
See this comment (https://github.com/esp8266/Arduino/blob/master/libraries/ESP8266WiFi/src/include/UdpContext.h#L443) (however "the former IPv4-only way suffers from the exact same issue" part needs fix, see below).

On 2.4.2 the address were taken from the packet header because only IPv4 was received and we had the IP address decoding macros (and a fixed-known-size of packet header)

Arduino/libraries/ESP8266WiFi/src/include/UdpContext.h

Line 162 in bb28d4a

uint32_t getRemoteAddress()

With IPv6 now around and newer lwIP, these macros are not valid anymore, and we don't have the packet decoding logic (which is not easily available outside from internal lwIP code).

Now instead, IP address is read at the time when the packet is received because this is only when it is safely available from lwIP (in our _recv callback), and stored in Udp class. This value is overwritten at every new incoming packet.

What can be done:

1. restore old behaviour when IPv6 is not enabled (pretty much inconsistent solution)
2. restore old behaviour when both IPv4 and IPv6 are enabled (require parsing IP packet, not our job but lwIP's job, could be done)
3. make a list/array/queue/stack/whatever of addresses to make them match the actual parsed packet (will be backward compatible, maybe a little heavy)
4. expose Udp.onReceive() that is implemented in UdpContext but not used. On a user callback, the remote address will always be correct (breaking api change).

Maybe we can do 3 the following way:
On 2.4.2, the packet header and IP address was read from before the beginning of payload. Payload is in the pbuf chain, but every pbuf (this has to be doubled-checked but that was the pre-2.5.0 IPv4 only way) has payload and header. We can't easily decode it anymore, but maybe we can overwrite the unused room in this header with the ip_address (given in _recv()) (thinking while writing :)

[devyte]

IMHO:

1. nope
2. maybe. The "if ipv4 then this else if ipv6 then that" bothers me, though.
3. if the described solution works for all versions, and is not specific to just one version, then I guess it's possible.
4. I like this one, I suspect it should be possible to build a solution that relies on onReceive(). No need to expose it, just use it internally. The wrapper would have the same interface as the current api.