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

Escape special characters in SSIDs and passwords when sending them to ESP32 #1604

Merged
merged 10 commits into from Nov 28, 2018

Conversation

Projects
None yet
3 participants
@sergeuz
Copy link
Member

sergeuz commented Nov 20, 2018

Problem

Argon fails to connect to a WiFi network if the network's password contains special characters (,, ", \).

Solution

Escape special characters in SSIDs and passwords as described in the AT command set manual (see the +CWJAP command).

Steps to Test

  • Change the password of your network to some string containing special characters, and make sure an Argon is able to connect to the network.

  • Run unit tests.

Example App

N/A

References


Completeness

  • User is totes amazing for contributing!
  • Contributor has signed CLA (Info here)
  • Problem and Solution clearly stated
  • Run unit/integration/application tests on device
  • Added documentation
  • Added to CHANGELOG.md after merging (add links to docs and issues)

@sergeuz sergeuz requested a review from m-mcgowan Nov 20, 2018

@sergeuz sergeuz added bug mesh labels Nov 20, 2018

const char c = src[i++];
if (memchr(spec, (unsigned char)c, specSize) != nullptr) {
if (j == destSize - 1) {
break; // Avoid generating an invalid escaped string

This comment has been minimized.

@m-mcgowan

m-mcgowan Nov 20, 2018

Contributor

How can the caller know the buffer was too small?

This comment has been minimized.

@sergeuz

sergeuz Nov 20, 2018

Member

There's no way to know that. We can make this function fill the entire output buffer and return the number of characters required to store the complete escaped string, which can be larger than the size of the output buffer – similarly to how printf() family of functions work. I just wasn't sure if it's necessary, given that the caller always knows the size of the buffer in the worst case (strlen(src) * 2 + 1)


namespace particle {

char* escape(const char* src, size_t srcSize, const char* spec, size_t specSize, char esc, char* dest, size_t destSize) {

This comment has been minimized.

@m-mcgowan

m-mcgowan Nov 20, 2018

Contributor

Could you add unit tests for this?

char* escape(const char* src, size_t srcSize, const char* spec, size_t specSize, char esc, char* dest, size_t destSize);

/**
* Escapes a set of characters by prepending them with an escape character.

This comment has been minimized.

@m-mcgowan

m-mcgowan Nov 20, 2018

Contributor

Nice to see the method is well documented. How does the caller know how many characters are added to the buffer. Is the output guaranteed to be null-terminated or only operates on the number of characters given?

For methods like this that are relatively simple, no side effects etc, I really liked doctest in python that allows you to write inline tests in documentation. Seems like there is an equivalent for C++. https://github.com/onqtam/doctest

@sergeuz

This comment has been minimized.

Copy link
Member

sergeuz commented Nov 26, 2018

Updated the implementation according to the suggestions and added unit tests: https://github.com/particle-iot/firmware/tree/fix/escape_ssid/test/unit_tests

@sergeuz sergeuz force-pushed the fix/escape_ssid branch from ca71d31 to 9533558 Nov 27, 2018

@sergeuz sergeuz requested a review from avtolstoy Nov 28, 2018

@sergeuz sergeuz merged commit 68ec187 into mesh-develop Nov 28, 2018

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@sergeuz sergeuz deleted the fix/escape_ssid branch Nov 28, 2018

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