Skip to content

WifiMulti.addAP code inspection : Incoherent test for max length of passphrase #2247

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
SteveToulouse opened this issue Jul 7, 2016 · 5 comments

Comments

@SteveToulouse
Copy link
Contributor

SteveToulouse commented Jul 7, 2016

Description

WifiMulti.addAP limits the passphrase to 63 characters by the test at L175.

On the other hand, Wifi.begin recognises a length of 64 chars as a PSK instead of a passphrase, see tests at L109 and L118.

This seems to be incoherent (wifimulti.run calls wifi.begin) unless we aren't allowed to pass PSK to wifimulti by design?

NOTE : SSID is handled the same in both places (31 char max). EDIT: the standard says 32 characters max for an SSID so there may be rare cases where an AP has a full 32 char, non zero-terminated SSID and in that case ESP8266 won't be able to connect to it.

@zenmanenergy
Copy link

According to wl_definitions.h it looks like it should be 63. Does that mean the WifiMulti.addAP is correct with 63 and the ESP8266WiFiSTA.cpp is incorrect with 64?

@devyte
Copy link
Collaborator

devyte commented Oct 15, 2017

Per Espressif API reference v2.1.2, section 7.2.1:
char password[64]

The above includes the null terminator, though.

@devyte devyte self-assigned this Dec 29, 2017
@devyte
Copy link
Collaborator

devyte commented Jan 1, 2018

It took me a while to fully understand what the correct behavior is supposed to be, and what is right/wrong in the code. At irst I thought this was just a boundary condition that needed fixing. As it turns out, there are two cases for passphrases: 63 char printable ascii with null terminator, and 64 byte hex with/without null terminator.
WiFi implements both cases, with certain assumptions for the 64byte hex case, while WiFiMulti implements only the 63 ascii case.
The Espressif SDK field for the passphrase is 64bytes in size, so the hex case must be handled in a special manner, and not as a null terminated string.
This is not an inconsistency or a bug, but a limitation in the implementation, of WiFiMulti, and is therefore an enhancement. Because there are certain assumptions made in WiFi, which are not quite clear, and which must be propagated, I consider the enhancement in WiFiMulti a bit delicate.
@igrr I suggest moving this to 2.5.0.

@igrr igrr modified the milestones: 2.4.0, 2.5.0 Jan 1, 2018
@devyte
Copy link
Collaborator

devyte commented Jan 3, 2018

@SteveToulouse @zenmanenergy could someone please test this?

@devyte
Copy link
Collaborator

devyte commented Jan 11, 2018

Closed via #4076 .

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

No branches or pull requests

4 participants