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

Fix handling of 0-length SSID in ScanNetworks #17060

Closed
bzbarsky-apple opened this issue Apr 5, 2022 · 3 comments · Fixed by #17197
Closed

Fix handling of 0-length SSID in ScanNetworks #17060

bzbarsky-apple opened this issue Apr 5, 2022 · 3 comments · Fixed by #17197
Labels
bug Something isn't working spec Mismatch between spec and implementation V1.0

Comments

@bzbarsky-apple
Copy link
Contributor

bzbarsky-apple commented Apr 5, 2022

The driver interface is not documented very well, so the implementations of it differ in how they handle the SSID. For example, src/platform/Ameba/NetworkCommissioningWiFiDriver.cpp checks ssid.data() to decide whether we're doing a single-ssid scan or general scan, while src/platform/EFR32/NetworkCommissioningWiFiDriver.cpp checks ssid.size(). If we checked the size in the caller and enforced the "1 to 32" length constraint, the difference would at least not be observable, but as it is they would reply differently to the same protocol message.

We should:

  1. Check the 1 to 32 length constraint in the cluster code and respond with an error if given a non-null 0-length SSID. Though maybe an empty SSID should be allowed here in spite of the constraint saying "1 to 32" because this is nullable? @CamWms
  2. Change the driver API to have separate functions for "scan for networks with the given SSID" and "scan for all networks" so the only place where we have to make that decision is in the cluster code and drivers can just do what they're told.

Originally posted by @bzbarsky-apple in #17038 (comment)

@bzbarsky-apple bzbarsky-apple added bug Something isn't working spec Mismatch between spec and implementation labels Apr 5, 2022
@CamWms
Copy link

CamWms commented Apr 5, 2022

For a list octstr, or string, zero length is synonymous with null (in DM spec).

@bzbarsky-apple
Copy link
Contributor Author

If we want that behavior, we can implement that too (need to sort out what the actual spec behavior here is). But that decision should be made in one place: in the cluster.

@bzbarsky-apple
Copy link
Contributor Author

Spec decision seems to be that the spec here should allow lengths 0-32, and 0 should be treated the same as null.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working spec Mismatch between spec and implementation V1.0
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants