Skip to content

catch error when http init fails from bad url to avoid barf. #48

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jun 17, 2025
Merged

catch error when http init fails from bad url to avoid barf. #48

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/remote.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,11 @@ int remote_get(const char* url, uint8_t** buf, size_t* len, uint8_t* brightness_
};

esp_http_client_handle_t http = esp_http_client_init(&config);
if (http == NULL) {
ESP_LOGE(TAG, "HTTP client initialization failed for URL: %s", url);
free(state.buf);
return 1;
}
Comment on lines +162 to +166
Copy link

Choose a reason for hiding this comment

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

medium

The addition of the null check for the http client handle is a crucial improvement for robustness, preventing potential crashes if esp_http_client_init fails (e.g., due to a malformed URL).

Regarding the free(state.buf); call on line 164:
For this operation to be safe and correct, state.buf must satisfy one of the following conditions at this point:

  1. It is NULL. Calling free(NULL) is a safe no-operation. This is typically the case if the state struct instance (which state.buf is a member of) was zero-initialized (e.g., http_state_t state = {0};) and state.buf has not yet been allocated by the HTTP event handler (which wouldn't have run if init failed).
  2. It is a valid pointer to heap-allocated memory that was provisioned before esp_http_client_init was called and now needs to be explicitly freed in this error path to prevent a memory leak.

If state.buf could hold an uninitialized garbage value (e.g., if the state struct was a stack variable that wasn't initialized), calling free() on it would result in undefined behavior.

Please ensure that the state variable is managed such that state.buf is always in a valid state (either NULL or a valid heap pointer) before this free() call. If state.buf is expected to be NULL here because it's only allocated within the event handler, the free() call is benignly redundant but safe if state was zero-initialized.


// Do the request
esp_err_t err = esp_http_client_perform(http);
Expand Down
3 changes: 1 addition & 2 deletions src/wifi.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,8 +142,7 @@ static void connect_to_ap(void);
static void url_decode(char *str);
static bool has_saved_config = false;
// Initialize WiFi
int
wifi_initialize(const char *ssid, const char *password) {
int wifi_initialize(const char *ssid, const char *password) {
ESP_LOGI(TAG, "Initializing WiFi");

// Initialize NVS
Expand Down