Skip to content

Commit

Permalink
refactor(esp_http_server): Improve httpd string value fetching effici…
Browse files Browse the repository at this point in the history
…ency

- Avoid using `strlcpy()` when source is not null-terminated;
- Avoid duplicate `strlen()` when `strlcpy()` is used.
  • Loading branch information
Adam5Wu committed Jan 26, 2025
1 parent c586527 commit aa0bfd2
Showing 1 changed file with 30 additions and 28 deletions.
58 changes: 30 additions & 28 deletions components/esp_http_server/src/httpd_parse.c
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,11 @@ static esp_err_t verify_url (http_parser *parser)
return ESP_FAIL;
}

/* Keep URI with terminating null character. Note URI string pointed
/* Copy URI and append terminating null character. Note URI string pointed
* by 'at' is not NULL terminated, therefore use length provided by
* parser while copying the URI to buffer */
strlcpy((char *)r->uri, at, (length + 1));
memcpy((char *)r->uri, at, length);
((char *)r->uri)[length] = '\0';
ESP_LOGD(TAG, LOG_FMT("received URI = %s"), r->uri);

/* Make sure version is HTTP/1.1 or HTTP/1.0 (legacy compliance purpose) */
Expand All @@ -95,7 +96,7 @@ static esp_err_t verify_url (http_parser *parser)

/* Parse URL and keep result for later */
http_parser_url_init(res);
if (http_parser_parse_url(r->uri, strlen(r->uri),
if (http_parser_parse_url(r->uri, length,
r->method == HTTP_CONNECT, res)) {
ESP_LOGW(TAG, LOG_FMT("http_parser_parse_url failed with errno = %d"),
parser->http_errno);
Expand Down Expand Up @@ -855,8 +856,7 @@ esp_err_t httpd_query_key_value(const char *qry_str, const char *key, char *val,
return ESP_ERR_INVALID_ARG;
}

const char *qry_ptr = qry_str;
const size_t buf_len = val_size;
const char *qry_ptr = qry_str;

while (strlen(qry_ptr)) {
/* Search for the '=' character. Else, it would mean
Expand Down Expand Up @@ -890,14 +890,16 @@ esp_err_t httpd_query_key_value(const char *qry_str, const char *key, char *val,
qry_ptr = val_ptr + strlen(val_ptr);
}

/* Update value length, including one byte for null */
val_size = qry_ptr - val_ptr + 1;
/* Query value length does not include terminating null */
size_t val_len = qry_ptr - val_ptr;

/* Copy value to the caller's buffer. */
strlcpy(val, val_ptr, MIN(val_size, buf_len));
size_t copy_len = MIN(val_len, val_size - 1);
memcpy(val, val_ptr, copy_len);
val[copy_len] = '\0';

/* If buffer length is smaller than needed, return truncation error */
if (buf_len < val_size) {
if (copy_len < val_len) {
return ESP_ERR_HTTPD_RESULT_TRUNC;
}
return ESP_OK;
Expand Down Expand Up @@ -953,12 +955,15 @@ esp_err_t httpd_req_get_url_query_str(httpd_req_t *r, char *buf, size_t buf_len)
if (res->field_set & (1 << UF_QUERY)) {
const char *qry = r->uri + res->field_data[UF_QUERY].off;

/* Minimum required buffer len for keeping
* null terminated query string */
size_t min_buf_len = res->field_data[UF_QUERY].len + 1;
/* Query data length does not include terminating null */
size_t data_len = res->field_data[UF_QUERY].len;

strlcpy(buf, qry, MIN(buf_len, min_buf_len));
if (buf_len < min_buf_len) {
/* Copy data to the caller's buffer. */
size_t copy_len = MIN(data_len, buf_len - 1);
memcpy(buf, qry, copy_len);
buf[copy_len] = '\0';

if (copy_len < data_len) {
return ESP_ERR_HTTPD_RESULT_TRUNC;
}
return ESP_OK;
Expand Down Expand Up @@ -1074,13 +1079,10 @@ esp_err_t httpd_req_get_hdr_value_str(httpd_req_t *r, const char *field, char *v
}

/* Get the NULL terminated value and copy it to the caller's buffer. */
strlcpy(val, val_ptr, buf_len);

/* Update value length, including one byte for null */
val_size = strlen(val_ptr) + 1;
size_t full_size = strlcpy(val, val_ptr, val_size);

/* If buffer length is smaller than needed, return truncation error */
if (buf_len < val_size) {
if (val_size < full_size) {
return ESP_ERR_HTTPD_RESULT_TRUNC;
}
return ESP_OK;
Expand All @@ -1096,8 +1098,6 @@ esp_err_t static httpd_cookie_key_value(const char *cookie_str, const char *key,
}

const char *cookie_ptr = cookie_str;
const size_t buf_len = *val_size;
size_t _val_size = *val_size;

while (strlen(cookie_ptr)) {
/* Search for the '=' character. Else, it would mean
Expand Down Expand Up @@ -1130,19 +1130,21 @@ esp_err_t static httpd_cookie_key_value(const char *cookie_str, const char *key,
cookie_ptr = val_ptr + strlen(val_ptr);
}

/* Update value length, including one byte for null */
_val_size = cookie_ptr - val_ptr + 1;
/* Cookie value length does not include terminating null */
size_t val_len = cookie_ptr - val_ptr;

/* Copy value to the caller's buffer. */
strlcpy(val, val_ptr, MIN(_val_size, buf_len));
size_t copy_len = MIN(val_len, *val_size - 1);
memcpy(val, val_ptr, copy_len);
val[copy_len] = '\0';

/* Save actual Cookie value size (including terminating null) */
*val_size = copy_len + 1;

/* If buffer length is smaller than needed, return truncation error */
if (buf_len < _val_size) {
*val_size = _val_size;
if (copy_len < val_len) {
return ESP_ERR_HTTPD_RESULT_TRUNC;
}
/* Save amount of bytes copied to caller's buffer */
*val_size = MIN(_val_size, buf_len);
return ESP_OK;
}
ESP_LOGD(TAG, LOG_FMT("cookie %s not found"), key);
Expand Down

0 comments on commit aa0bfd2

Please sign in to comment.