Add support for long and unconventional SSIDs and passwords #15
Conversation
TilCreator
changed the title
wifimgr.py
Outdated
| s = r[i] | ||
| try: | ||
| r[i] = chr(int(s[:2], 16)) + s[2:] | ||
| except Exception: |
There was a problem hiding this comment.
please do not add new code with except: or except Exception:, rather catch the more specific exceptions.
wifimgr.py
Outdated
| @@ -15,6 +15,17 @@ | |||
| server_socket = None | |||
|
|
|||
|
|
|||
| def _unquote(s): | |||
There was a problem hiding this comment.
hmm, isn't there something in urllib (or elsewhere) for that already?
There was a problem hiding this comment.
In cPython it's possible to do that with urllib, but Micropython doesn't have urllib and apparently nothing similar.
There was a problem hiding this comment.
ok, in that case one could consider copying quote/unquote from the cpython stdlib - just to avoid issues in new code and maybe be able to use it from some lib later in case micropython adds that lib or there is some external lib on pypi (use same interface).
There was a problem hiding this comment.
I've just tried that and I don't think it would be a good idea, because the function depends on about 70 lines of code, it needs more resources and had to be modified to run on Micropython. What I think would be better to just rename the function to the urllib counterpart ( unquote_plus docs ).
There was a problem hiding this comment.
ok, if you can support the same interface and semantics, using the same name makes sense.
wifimgr.py
Outdated
| password = match.group(2).replace("%3F", "?").replace("%21", "!") | ||
|
|
||
| ssid = _unquote(match.group(1).decode("utf-8").replace("%3F", "?").replace("%21", "!")) | ||
| password = _unquote(match.group(2).decode("utf-8").replace("%3F", "?").replace("%21", "!")) |
There was a problem hiding this comment.
why did you keep the replace for the %XX stuff here, isn't that something your new _unquote call should do?
wifimgr.py
Outdated
| password = _unquote(match.group(2).decode("utf-8").replace("%3F", "?").replace("%21", "!")) | ||
| except: | ||
| ssid = _unquote(match.group(1).replace("%3F", "?").replace("%21", "!")) | ||
| password = _unquote(match.group(2).replace("%3F", "?").replace("%21", "!")) |
There was a problem hiding this comment.
same here. also a new diaper pattern (using a bare except:).
wifimgr.py
Outdated
| @@ -281,18 +291,31 @@ def start(port=80): | |||
| print('client connected from', addr) | |||
| try: | |||
| client.settimeout(5.0) | |||
|
|
|||
| request = b"" | |||
| request = bytearray(b"") | |||
There was a problem hiding this comment.
just bytearray() also creates an empty one.
wifimgr.py
Outdated
|
|
||
| while content_length_remaining > 0: | ||
| chunk = client.recv(512) | ||
| request.extend(chunk) |
There was a problem hiding this comment.
as you already have the starting of the content in content, wouldn't you rather like to append the rest of the content to that instead of to request?
There was a problem hiding this comment.
or at least remember that position of \r\n\r\n (end of headers) for later, so the body can be extracted more easily.
wifimgr.py
Outdated
|
|
||
| ssid = _unquote(match.group(1).decode("utf-8")) | ||
| password = _unquote(match.group(2).decode("utf-8")) | ||
| except: |
There was a problem hiding this comment.
you didn't fix this one yet.
except: is the worst (google for "diaper pattern") and every linter and also human python coders might puke on it. :-)
except Exception: is a little better, at least some linters will shut up. But still, it is not as specific as usually desired and might catch some unintended exceptions.
There was a problem hiding this comment.
I didn't fixed it jet, because it's from the old / original code, but I will fix it. Let me just check which exception gets triggered.
There was a problem hiding this comment.
The old code (as seen in the diff) had except Exception:, so by changing it to except: you made it slightly worse.
Thanks for checking what exceptions actually should get catched there, this is what one really wants to do. :)
There was a problem hiding this comment.
Ups, I changed it in #14 and then falsely merged it, my bad.
This makes sure that all data is received and correctly decoded.