SuperHouse / esp-open-rtos

Open source FreeRTOS-based ESP8266 software framework
BSD 3-Clause "New" or "Revised" License
1.52k stars 491 forks source link

sdk_wifi_station_scan not working properly for 32 character SSID #682

Open ja2142 opened 5 years ago

ja2142 commented 5 years ago

I'm not sure if this is a bug or not, but sdk_wifi_station_scan doesn't seem to work properly for 32 character SSID's. Example output from wifi_scan example:

----------------------------------------------------------------------------------
                             Wi-Fi networks
----------------------------------------------------------------------------------
                    UPC240176871 (70:85:c6:e5:5b:7e) RSSI: -87, security: WPA2/PSK
7GPowerOverWifi_MSDos_compatible␆�␃ (38:43:7d:8e:3a:32) RSSI: -44, security: WPA2/PSK
  012345678901234567890123456789 (b0:89:00:22:00:48) RSSI: -42, security: WPA2/PSK

Second wifi name should be "7GPowerOverWifi_MSDos_compatible", but there are extra unprintable characters at the end.

Connection to said wifi works fine, but the extra characters are printed on connection:

connected with 7GPowerOverWifi_MSDos_compatible␃␅␃, channel 6

One of the causes is probably in include/espressif/esp_sta.h:

struct sdk_station_config {
    uint8_t ssid[32];     /* Null terminated string */
    uint8_t password[64]; /* Null terminated string */
    uint8_t bssid_set;    /* One if bssid is used, otherwise zero. */
    uint8_t bssid[6];     /* The BSSID bytes */
};

All ssids are stored as uint8_t[32], which means that the 32 byte ssid shouldn't fit.

Another thing is the fact that in wifi_scan example any characters after 32 should be cut by: printf("%32s (" MACSTR ") RSSI: %02d, security: %s\n", ssid, MAC2STR(bss->bssid), bss->rssi, auth_modes[bss->authmode]); This means that printf string limiting isn't working. Indeed printf("%2s","0123456789"); outputs 0123456789 However %.*s format seems to be working as expected.

virangjhaveri commented 5 years ago

printf function prints until '\0' is found. Since the SSID is exact 32 and the char array to store is also 32, the print function goes beyond and prints from memory location after the end of the variable. Hence the garbage characters are printed.

ja2142 commented 5 years ago

I know how printf works and probably why the extra characters are printed. My problems are:

  1. From to my understanding 32-byte SSIDs should be supported (they seem to be working with my router, 5 PCs under both Windows and Linux, wireless printer and ESP itself when it comes to connecting). As I said ESP is connecting with the WiFi, but sdk_wifi_station_scan gives me the extra characters which shoudn't be there.
  2. Cutting printf format eg. printf("%32s",str); is meant to print only 32 characters from str. This doesn't work, and this fact may be related.

I guess that ssid string is only null terminated if it has fewer then 32 bytes, and for length of 32 it relies on cutting it by "%32s" but i have no idea how to check or repair it.

phkehl commented 5 years ago

"%32s" will pad 32 characters (spaces), but it will still print more than 32 characters if the string is longer. Copy the string into a temporary string and nul-terminate it yourself:

char str[33];
strncpy(str, ssid, 32);
str[32] = '\0';
printf("ssid=[%s]", str);
ja2142 commented 5 years ago

Ok, so I was mistaken with printf format for cutting.

This doesn't change the fact that sdk_wifi_station_scan gives too long strings for 32 byte SSIDs. I know that I could cut the extra bit elsewhere, but I belive that I shouldn't have to do that, because that's what library is for.

That being said sdk_wifi_station_scan is provided as a binary and I have no idea how to modify it.

phkehl commented 5 years ago

Yeah, well. There are weirder things than this one. When I stumbled over this my thought was that the "null terminated string" comment in the struct sdk_station_confg is just wrong. It's an array of bytes, not a string.

Edit: remove scan function I posted here earlier, it was from different SDK. I'll post a copy of my scan function for the esp-open-rtos later.

ja2142 commented 5 years ago

So it seems that ssids in sdk are null terminated only when less then 32 bytes in length. This leaves the user with limiting ssid's length. That's fine, but then the usages are wrong - two that i know of are wifi_scan example and cgiWifiScan from libesphttpd/util/cgiwifi.c. Example is harmless, but libesphttpd case adds some characters and the characters added may be illegal for JSON and make it unparsable (which happens for me sometimes). To fix this issue I guess I should do PR to https://github.com/nochkin/libesphttpd, right? Or should I PR original libesphttpd https://github.com/Spritetm/libesphttpd