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

IPAddress construction and comparison are buggy on Arduino 3 #9724

Closed
1 task done
mathieucarbou opened this issue May 29, 2024 · 1 comment · Fixed by #9725
Closed
1 task done

IPAddress construction and comparison are buggy on Arduino 3 #9724

mathieucarbou opened this issue May 29, 2024 · 1 comment · Fixed by #9725
Labels
Status: Awaiting triage Issue is waiting for triage

Comments

@mathieucarbou
Copy link
Contributor

mathieucarbou commented May 29, 2024

Board

N/A

Device Description

N/A

Hardware Configuration

N/A

Version

v3.0.0

IDE Name

N/A

Operating System

N/A

Flash frequency

N/A

PSRAM enabled

no

Upload speed

N/A

Description

The origin of this bug request comes for a bug report in my ESPAsyncWebServer maintained fork here: mathieucarbou/ESPAsyncWebServer#26

The root of the problem lies in the fact that when connected to a WiFi, both should be true.

WiFi.localIP().toString() == request->client()->localIP().toString()
WiFi.localIP() == request->client()->localIP()

But the second one is false.

This is because the library uses behind the new IPAddress construction with an ip_addr_t (following some updates from the ESPHome team). This is not wrong, but the new implementation has flaws.

The issue lies at several places:

  1. the comparison operator
bool IPAddress::operator==(const IPAddress &addr) const {
  return (addr._type == _type) && (memcmp(addr._address.bytes, _address.bytes, sizeof(_address.bytes)) == 0);
}

For IPv4, it should only compare _address.dword[IPADDRESS_V4_DWORD_INDEX], not the full set of bytes

  1. the constructor:
IPAddress &IPAddress::from_ip_addr_t(const ip_addr_t *addr) {
  if (addr->type == IPADDR_TYPE_V6) {
    _type = IPv6;
    _address.dword[0] = addr->u_addr.ip6.addr[0];
    _address.dword[1] = addr->u_addr.ip6.addr[1];
    _address.dword[2] = addr->u_addr.ip6.addr[2];
    _address.dword[3] = addr->u_addr.ip6.addr[3];
#if LWIP_IPV6_SCOPES
    _zone = addr->u_addr.ip6.zone;
#endif /* LWIP_IPV6_SCOPES */
  } else {
    _type = IPv4;
    _address.dword[IPADDRESS_V4_DWORD_INDEX] = addr->u_addr.ip4.addr;
  }
  return *this;
}

Which is not "clearing" the indexes 0, 1 and 2 to 0, to ensure the comparison operator works for IPv4

Any of this fix would work. I tested this one:

IPAddress &IPAddress::from_ip_addr_t(const ip_addr_t *addr) {
  if (addr->type == IPADDR_TYPE_V6) {
    _type = IPv6;
    _address.dword[0] = addr->u_addr.ip6.addr[0];
    _address.dword[1] = addr->u_addr.ip6.addr[1];
    _address.dword[2] = addr->u_addr.ip6.addr[2];
    _address.dword[3] = addr->u_addr.ip6.addr[3];
#if LWIP_IPV6_SCOPES
    _zone = addr->u_addr.ip6.zone;
#endif /* LWIP_IPV6_SCOPES */
  } else {
    _type = IPv4;
    _address.dword[0] = 0;
    _address.dword[1] = 0;
    _address.dword[2] = 0;
    _address.dword[IPADDRESS_V4_DWORD_INDEX] = addr->u_addr.ip4.addr;
  }
  return *this;
}

But I think both should be applied (also making sure that the compare operator is comparing the relevant information depending on the IP type)

Sketch

N/A

Debug Message

N/A

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.
@mathieucarbou
Copy link
Contributor Author

Note: the workaround could be to not use the constructor and proceed like that:

IPAddress ip;
ip.from_ip_addr_t(ipaddr);
connect(ip, _connect_port);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Awaiting triage Issue is waiting for triage
Projects
None yet
1 participant