Skip to content

My first change #1409

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

Open
yazdan-iot wants to merge 6 commits into
base: master
Choose a base branch
from
Open

My first change #1409

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ed48071
First Commit
abolfazl33369 Jun 29, 2026
ee670ea
Fix: check malloc() result in fnHttpClient to prevent null pointer crash
yazdan-iot Jun 30, 2026
478fbd3
Remove accidental test text from README
yazdan-iot Jun 30, 2026
37718b3
Fix 7 bugs in lib/http/
yazdan-iot Jul 2, 2026
0f9c1fc
Use strlcpy instead of strncpy for null termination
yazdan-iot Jul 2, 2026
e19196e
Add compat_string.h include for strlcpy
yazdan-iot Jul 2, 2026
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
23 changes: 18 additions & 5 deletions lib/http/fnHttpClient.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ const char *webdav_depths[] = {"0", "1", "infinity"};
fnHttpClient::fnHttpClient()
{
_buffer = (char *)malloc(DEFAULT_HTTP_BUF_SIZE);
if (_buffer == nullptr)
{
Debug_printf("fnHttpClient::fnHttpClient() failed to allocate %d byte buffer\r\n", DEFAULT_HTTP_BUF_SIZE);
}
Comment on lines 27 to 30

@FozzTexx FozzTexx Jul 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm pretty sure this fix is already in master. You probably need to rebase this branch on master. First go to your fork and go to your master branch and click the Sync fork button. Then from your cloned repo:

git checkout master
git pull
git checkout my-first-change
git rebase master
git push --force-with-lease

@yazdan-iot yazdan-iot Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ok thanks

}

// Close connection, destroy any resoruces
Expand Down Expand Up @@ -293,7 +297,7 @@ esp_err_t fnHttpClient::_httpevent_handler(esp_http_client_event_t *evt)
Debug_printf("HTTP_EVENT_ON_HEADER %u\r\n", uxTaskGetStackHighWaterMark(nullptr));
#endif
// Check to see if we should store this response header
if (client->_stored_headers.size() <= 0)
if (client->_stored_headers.size() == 0)
break;

client->set_header_value(evt->header_key, evt->header_value);
Expand Down Expand Up @@ -354,9 +358,16 @@ esp_err_t fnHttpClient::_httpevent_handler(esp_http_client_event_t *evt)
Debug_printf("HTTP_EVENT_ON_DATA: Data: %p, Datalen: %d\r\n", evt->data, evt->data_len);
#endif

client->_buffer_pos = 0;
client->_buffer_len = (evt->data_len > DEFAULT_HTTP_BUF_SIZE) ? DEFAULT_HTTP_BUF_SIZE : evt->data_len;
memcpy(client->_buffer, evt->data, client->_buffer_len);
if (client->_buffer == nullptr) {
Debug_printf("HTTP_EVENT_ON_DATA: _buffer is null, dropping data\r\n");
client->_buffer_pos = 0;
client->_buffer_len = 0;
}
else {
client->_buffer_pos = 0;
client->_buffer_len = (evt->data_len > DEFAULT_HTTP_BUF_SIZE) ? DEFAULT_HTTP_BUF_SIZE : evt->data_len;
memcpy(client->_buffer, evt->data, client->_buffer_len);
}

// Now let the reader know there's data in the buffer
xTaskNotifyGive(client->_taskh_consumer);
Expand Down Expand Up @@ -811,7 +822,9 @@ char *fnHttpClient::get_header(int index, char *buffer, int buffer_len)

auto vi = _stored_headers.begin();
std::advance(vi, index);
return strncpy(buffer, vi->second.c_str(), buffer_len);
strncpy(buffer, vi->second.c_str(), buffer_len - 1);
buffer[buffer_len - 1] = '\0';
return buffer;
Comment on lines -814 to +827

@FozzTexx FozzTexx Jul 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It's easier to replace strncpy() with strlcpy() which will guarantee it's null terminated.

}

const std::string fnHttpClient::get_header(int index)
Expand Down
14 changes: 11 additions & 3 deletions lib/http/httpService.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,11 @@ char to_hex(char code)
/* IMPORTANT: be sure to free() the returned string after use */
char *url_encode(char *str)
{
char *pstr = str, *buf = (char *)malloc(strlen(str) * 3 + 1), *pbuf = buf;
char *pstr = str;
char *buf = (char *)malloc(strlen(str) * 3 + 1);
if (buf == NULL)
return NULL;
char *pbuf = buf;
while (*pstr)
{
if (isalnum(*pstr) || *pstr == '-' || *pstr == '_' || *pstr == '.' || *pstr == '~')
Expand All @@ -84,7 +88,11 @@ char *url_encode(char *str)
/* IMPORTANT: be sure to free() the returned string after use */
char *url_decode(char *str)
{
char *pstr = str, *buf = (char *)malloc(strlen(str) + 1), *pbuf = buf;
char *pstr = str;
char *buf = (char *)malloc(strlen(str) + 1);
if (buf == NULL)
return NULL;
char *pbuf = buf;
while (*pstr)
{
if (*pstr == '%')
Expand Down Expand Up @@ -341,7 +349,7 @@ void fnHttpService::parse_query(httpd_req_t *req, queryparts *results)
}

/// @todo Error if path_end == 0, the index to substr becomes -1
results->path += results->full_uri.substr(0, path_end - 1);
results->path += results->full_uri.substr(0, path_end);
results->query += results->full_uri.substr(path_end + 1);

// URL Decode query
Expand Down
2 changes: 1 addition & 1 deletion lib/http/httpServiceParser.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -652,7 +652,7 @@ const string fnHttpServiceParser::substitute_tag(const string &tag)
{
strncat(result, "<option value=\"", MAX_PRINTER_LIST_BUFFER-1);
strncat(result, PRINTER_CLASS::printer_model_str[i], MAX_PRINTER_LIST_BUFFER-1);
strncat(result, "\">", MAX_PRINTER_LIST_BUFFER);
strncat(result, "\">", MAX_PRINTER_LIST_BUFFER - strlen(result) - 1);
strncat(result, PRINTER_CLASS::printer_model_str[i], MAX_PRINTER_LIST_BUFFER-1);
strncat(result, "</option>\n", MAX_PRINTER_LIST_BUFFER-1);
}
Expand Down
6 changes: 4 additions & 2 deletions lib/http/mgHttpClient.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -984,7 +984,7 @@ int mgHttpClient::COPY(const char *destination, bool overwrite, bool move)
_flush_response();

// Set method
_method = HTTP_MOVE;
_method = move ? HTTP_MOVE : HTTP_COPY;
// Set detination
set_header("Destination", destination);
// Set overwrite
Expand Down Expand Up @@ -1110,7 +1110,9 @@ char *mgHttpClient::get_header(int index, char *buffer, int buffer_len)

auto vi = _stored_headers.begin();
std::advance(vi, index);
return strncpy(buffer, vi->second.c_str(), buffer_len);
strncpy(buffer, vi->second.c_str(), buffer_len - 1);
buffer[buffer_len - 1] = '\0';
return buffer;

@FozzTexx FozzTexx Jul 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same here, just use strlcpy()

}

const std::string mgHttpClient::get_header(int index)
Expand Down
Loading