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

Add support for long and unconventional SSIDs and passwords #15

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
51 changes: 37 additions & 14 deletions wifimgr.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,17 @@
server_socket = None


def _unquote(s):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, isn't there something in urllib (or elsewhere) for that already?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In cPython it's possible to do that with urllib, but Micropython doesn't have urllib and apparently nothing similar.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 ).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, if you can support the same interface and semantics, using the same name makes sense.

r = s.replace('+', ' ').split('%')
for i in range(1, len(r)):
s = r[i]
try:
r[i] = chr(int(s[:2], 16)) + s[2:]
except ValueError:
r[i] = '%' + s
return ''.join(r)


def get_connection():
"""return a working WLAN(STA_IF) instance or None"""

Expand Down Expand Up @@ -178,20 +189,19 @@ def handle_root(client):
client.close()


def handle_configure(client, request):
match = ure.search("ssid=([^&]*)&password=(.*)", request)
def handle_configure(client, content):
match = ure.search("ssid=([^&]*)&password=(.*)", content)

if match is None:
send_response(client, "Parameters not found", status_code=400)
return False
# version 1.9 compatibility
try:
ssid = match.group(1).decode("utf-8").replace("%3F", "?").replace("%21", "!")
password = match.group(2).decode("utf-8").replace("%3F", "?").replace("%21", "!")
except Exception:
ssid = match.group(1).replace("%3F", "?").replace("%21", "!")
password = match.group(2).replace("%3F", "?").replace("%21", "!")

ssid = _unquote(match.group(1).decode("utf-8"))
password = _unquote(match.group(2).decode("utf-8"))
except:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ups, I changed it in #14 and then falsely merged it, my bad.

ssid = _unquote(match.group(1))
password = _unquote(match.group(2))
if len(ssid) == 0:
send_response(client, "SSID must be provided", status_code=400)
return False
Expand Down Expand Up @@ -281,18 +291,31 @@ def start(port=80):
print('client connected from', addr)
try:
client.settimeout(5.0)

request = b""
request = bytearray()
try:
while "\r\n\r\n" not in request:
request += client.recv(512)
request.extend(client.recv(512))
except OSError:
pass

print("Request is: {}".format(request))
if "HTTP" not in request: # skip invalid requests
if "HTTP" not in request:
# skip invalid requests
continue

if "POST" in request and "Content-Length: " in request:
content_length = int(ure.search("Content-Length: ([0-9]+)?", bytes(request)).group(1))
content = bytearray(request[bytes(request).index(b"\r\n\r\n") + 4:])
content_length_remaining = content_length - len(content)

while content_length_remaining > 0:
chunk = client.recv(512)
content.extend(chunk)
content_length_remaining -= len(chunk)

request = bytes(request)

print("Request is: {}".format(request))

# version 1.9 compatibility
try:
url = ure.search("(?:GET|POST) /(.*?)(?:\\?.*?)? HTTP", request).group(1).decode("utf-8").rstrip("/")
Expand All @@ -303,7 +326,7 @@ def start(port=80):
if url == "":
handle_root(client)
elif url == "configure":
handle_configure(client, request)
handle_configure(client, bytes(content))
else:
handle_not_found(client, url)

Expand Down