MycroftAI / mycroft-wifi-setup

Mycroft WiFi Setup Client
GNU General Public License v3.0
4 stars 10 forks source link

WiFi Setup fails if network name has an apostrophe #42

Open augray opened 3 years ago

augray commented 3 years ago

Describe the bug My network name has an apostrophe ('). When trying to do the auto-network setup on my Mark 1, it would crash (from my phone, which I used to connect to start.mycroft.ai, it looked like it just spun on the network loading page forever).

To Reproduce Steps to reproduce the behavior:

  1. Name your WiFi network with an apostrophe in the name
  2. Run mycroft-wifi-setup

Expected behavior I would expect the networks to load so I can select my WiFi network and hook Mycroft up to it.

Log files

2021-08-15 02:56:27,685 - websocket - ERROR - error from callback <bound method WifiClient.on_message of <wifisetup.wifi_client.WifiClient object at 0x765dbd90>>: invalid syntax (<unknown>, line 1)
  File "websocket/_app.py", line 320, in _callback
  File "wifisetup/wifi_client.py", line 96, in on_message
  File "wifisetup/wifi_client.py", line 185, in scan
  File "ast.py", line 46, in literal_eval
  File "ast.py", line 35, in parse

From the wifi setup util ^

Environment (please complete the following information):

Additional context

The problem is this line where y'all try to get a python string by using a convoluted literal eval. A network name like augray's network becomes the python code b'augray's network' which then gets literal eval'ed. Python string/byte encoding may be hard, but it's not that hard 😉.

MatthewScholefield commented 3 years ago

Hmm, good catch. Also, note that the hacky literal eval is actually I think because the actual ssid string we get might be written as the literal string1\x2b1 and so we need literal_eval to parse hex escape codes into corresponding characters.

Just mentioning this to explain why the solution might actually be just to escape apostrophes despite how horridly hacky that seems xD.

augray commented 3 years ago

Gotcha, that’s gnarly 😛.

krisgesling commented 3 years ago

Hi Augray, thanks for filing a bug report however I'm pretty doubtful that we'll get time to work on this.

This package isn't compatible with the newer OS's so it's only in use on the Mark 1.

I've added a "help wanted" flag in case anyone else wants to tackle it. Just wanted to be open about what you can expect from us.

augray commented 3 years ago

Fine by me, I’ve got things working for myself (just renamed the network). I just wanted to report it in case you were still using this for the Mark II

On Sun, Aug 15, 2021 at 7:31 PM Kris Gesling @.***> wrote:

Hi Augray, thanks for filing a bug report however I'm pretty doubtful that we'll get time to work on this.

This package isn't compatible with the newer OS's so it's only in use on the Mark 1.

I've added a "help wanted" flag in case anyone else wants to tackle it. Just wanted to be open about what you can expect from us.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/MycroftAI/mycroft-wifi-setup/issues/42#issuecomment-899171530, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABWZKFOMYKBQV6NZTIIZSWDT5BZ65ANCNFSM5CFYGBJA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email .

mcdonc commented 3 years ago

Hmm, good catch. Also, note that the hacky literal eval is actually I think because the actual ssid string we get might be written as the literal string1\x2b1 and so we need literal_eval to parse hex escape codes into corresponding characters.

Just mentioning this to explain why the solution might actually be just to escape apostrophes despite how horridly hacky that seems xD.

Just a note here to myself that the "wifi" Python package scrapes the output of "/sbin/iwlist scan", and I think Matthew's assertion is that if an SSID has non-ASCII characters in it, those characters will be output as utf-8 encoded codepoints by iwlist, and that these codepoints will be retained as codepoints (not as unicode characters) even through the call iwlist_scan = iwlist_scan.decode('utf-8') in wifi.scan:Cell.all at https://github.com/rockymeza/wifi/blob/master/wifi/scan.py#L43. I'll try to set up a wifi environment to test the assertion once I have access to some hardware.

mcdonc commented 3 years ago

Indeed, with an ssid '🤔', Cell.ssid is '\xF0\x9F\xA4\x94' (and the ssid is not of type binary, it's a string).

This is a bug in the (now-unmaintained) wifi Python package, linked above, which should read the output of iwlist as binary, then decode various parts of it to strings instead of treating the entire output of iwlist scan as something that is itself a string.

The best way to fix it would likely to be to use a different library, or steal, fix, and inline the necessary code from the wifi package. But the pragmatic way is to utf-8 quote apostrophes (and double quotes, likely) as Matthew mentioned, in the ssid before passing it to ast.literal_eval.