Clean up CORS implemention #662
Conversation
eandersson
commented
Jan 17, 2025
eandersson
commented
- Use the internal AP state for CORS.
- Made ip_in_private_range a bit more readable.
- Re-factored code a bit.
|
|
||
| static esp_err_t is_network_allowed(httpd_req_t * req) | ||
| { | ||
| if (GLOBAL_STATE->SYSTEM_MODULE.ap_enabled == true) { |
There was a problem hiding this comment.
We should probably not use our own variable to keep the state as it can be error prone if someone forgets to call it, instead we should call the esp_wifi_get_mode fn, which is the source of truth.
There was a problem hiding this comment.
is there a good reason to keep a shadow copy of the AP state?
There was a problem hiding this comment.
Yep - I agree, but as a compromise for performance re-using the existing variable should be significantly more performant. Since it is directly controlled by the WiF component the risk that it isn't updated should be pretty low.
There was a problem hiding this comment.
There was a problem hiding this comment.
To chime in late, did you time the function call? As I think it's very unlikely it would cause a significant overhead compared to do a full HTTP request. Sounds like premature optimisation to me. And as you all know, Premature Optimisation Is the Root of All Evil.
There was a problem hiding this comment.
We can add a utility function in the future to add this back, but it was also fixing a bug in the code. For me it was mostly around using the same source of truth for all our code.
|
confirmed iOS setup AP works, and CSRF doesn't 👍 |
