From f4b7d50d8aea9b35631874682966f92f986b4ade Mon Sep 17 00:00:00 2001 From: Miroslav Zagorac <mzagorac@haproxy.com> Date: Mon, 17 Aug 2020 14:21:52 +0200 Subject: [PATCH] BUG/MINOR: fixed saving of HTTP headers data in struct mirror Sometimes, the header list was not initialized, so when the memory was freed in the mir_ptr_free() function, the program crashed because the list pointer had NULL content. This should probably solve the GitHub issue #12. Version of the program changed to v1.2.13. --- VERSION | 2 +- include/types/spoa-message.h | 2 +- src/.build-counter | 2 +- src/curl.c | 4 +-- src/spoa-message.c | 51 ++++++++++++++++++------------------ 5 files changed, 30 insertions(+), 31 deletions(-) diff --git a/VERSION b/VERSION index f2ae0b4..0b1f1ed 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.2.12 +1.2.13 diff --git a/include/types/spoa-message.h b/include/types/spoa-message.h index cd1d7e8..abf1a3a 100644 --- a/include/types/spoa-message.h +++ b/include/types/spoa-message.h @@ -41,7 +41,7 @@ struct mirror { char *method; /* */ int request_method; /* */ char *version; /* */ - struct list *hdrs; /* */ + struct list hdrs; /* */ char *body; /* */ size_t body_head; /* */ size_t body_size; /* */ diff --git a/src/.build-counter b/src/.build-counter index 3d212cf..e576ab3 100644 --- a/src/.build-counter +++ b/src/.build-counter @@ -1 +1 @@ -2415 +2432 diff --git a/src/curl.c b/src/curl.c index 3c18ada..2d02671 100644 --- a/src/curl.c +++ b/src/curl.c @@ -475,7 +475,7 @@ static CURLcode mir_curl_set_headers(struct curl_con *con, const struct mirror * if (_NULL(con) || _NULL(mir)) return retval; - list_for_each_entry_safe(hdr, hdr_back, mir->hdrs, list) { + list_for_each_entry_safe(hdr, hdr_back, &(mir->hdrs), list) { slist = curl_slist_append(con->hdrs, (const char *)hdr->ptr); if (_NULL(slist)) { retval = CURLE_OUT_OF_MEMORY; @@ -930,7 +930,7 @@ int mir_curl_add(struct curl_data *curl, struct mirror *mir) if (_NULL(curl) || _NULL(mir)) return retval; - CURL_DBG("Adding mirror { \"%s\" \"%s\" \"%s\" %d \"%s\" %p %p %zu/%zu }", mir->url, mir->path, mir->method, mir->request_method, mir->version, mir->hdrs, mir->body, mir->body_head, mir->body_size); + CURL_DBG("Adding mirror { \"%s\" \"%s\" \"%s\" %d \"%s\" { %p %p } %p %zu/%zu }", mir->url, mir->path, mir->method, mir->request_method, mir->version, mir->hdrs.p, mir->hdrs.n, mir->body, mir->body_head, mir->body_size); con_timeout_ms = CLAMP_VALUE(con_timeout_ms, CURL_CON_TMOUT_MIN, CURL_CON_TMOUT_MAX); timeout_ms = CLAMP_VALUE(timeout_ms, CURL_TMOUT_MIN, CURL_TMOUT_MAX); diff --git a/src/spoa-message.c b/src/spoa-message.c index d4a3999..aae191c 100644 --- a/src/spoa-message.c +++ b/src/spoa-message.c @@ -245,6 +245,7 @@ int spoa_msg_test(struct spoe_frame *frame, const char **buf, const char *end) * frame - * buf - * end - + * hdrs - * * DESCRIPTION * - @@ -252,25 +253,19 @@ int spoa_msg_test(struct spoe_frame *frame, const char **buf, const char *end) * RETURN VALUE * - */ -static struct list *spoa_msg_arg_hdrs(struct spoe_frame *frame, const char *buf, const char *end) +static int spoa_msg_arg_hdrs(struct spoe_frame *frame, const char *buf, const char *end, struct list *hdrs) { struct buffer *hdr = NULL, *hdr_back; - struct list *retptr = NULL; const char *str; uint64_t len; - int i, rc; + int i, retval = FUNC_RET_ERROR; - DBG_FUNC(FW_PTR, "%p, %p, %p", frame, buf, end); - - if (_NULL(retptr = calloc(1, sizeof(*retptr)))) - return retptr; - - LIST_INIT(retptr); + DBG_FUNC(FW_PTR, "%p, %p, %p, %p", frame, buf, end, hdrs); /* Build the HTTP headers. */ for (i = 0; buf < end; i++) { - rc = spoe_decode(frame, &buf, end, SPOE_DEC_STR0, &str, &len, SPOE_DEC_END); - if (_ERROR(rc)) + retval = spoe_decode(frame, &buf, end, SPOE_DEC_STR0, &str, &len, SPOE_DEC_END); + if (_ERROR(retval)) break; F_DBG(SPOA, frame, "str[%d]: <%.*s>", i, (int)len, str); @@ -278,22 +273,22 @@ static struct list *spoa_msg_arg_hdrs(struct spoe_frame *frame, const char *buf, if (i & 1) { if (_NULL(str)) { /* HTTP header has no value. */ - if (_ERROR(rc = buffer_grow(hdr, ";\0", 2))) + if (_ERROR(retval = buffer_grow(hdr, ";\0", 2))) break; } - else if (_ERROR(rc = buffer_grow_va(hdr, ": ", 2, str, len, "\0", 1, NULL))) { + else if (_ERROR(retval = buffer_grow_va(hdr, ": ", 2, str, len, "\0", 1, NULL))) { break; } F_DBG(SPOA, frame, "header[%d]: <%.*s>", i / 2, (int)hdr->len, hdr->ptr); - LIST_ADDQ(retptr, &(hdr->list)); + LIST_ADDQ(hdrs, &(hdr->list)); } else if (_NULL(str)) { if (buf != end) { /* HTTP header has no name. */ f_log(frame, _E("HTTP header defined without a name")); - rc = FUNC_RET_ERROR; + retval = FUNC_RET_ERROR; } break; @@ -304,16 +299,18 @@ static struct list *spoa_msg_arg_hdrs(struct spoe_frame *frame, const char *buf, } /* In the case of a fault, the allocated memory is released. */ - if (_ERROR(rc) || _NULL(hdr)) { + if (_ERROR(retval) || _NULL(hdr)) { buffer_ptr_free(&hdr); - list_for_each_entry_safe(hdr, hdr_back, retptr, list) + list_for_each_entry_safe(hdr, hdr_back, hdrs, list) { + LIST_DEL(&(hdr->list)); buffer_ptr_free(&hdr); + } - PTR_FREE(retptr); + retval = FUNC_RET_ERROR; } - return retptr; + return retval; } @@ -349,6 +346,7 @@ int spoa_msg_mirror(struct spoe_frame *frame, const char **buf, const char *end) return retval; } + LIST_INIT(&(mir->hdrs)); retval = spoe_decode(frame, &ptr, end, SPOE_DEC_UINT8, &nbargs, SPOE_DEC_END); if (_nERROR(retval)) @@ -386,13 +384,13 @@ int spoa_msg_mirror(struct spoe_frame *frame, const char **buf, const char *end) F_DBG(SPOA, frame, "mirror[%d] name='%.*s' type=%hhu: <%s> <%s>", i, (int)len, str, type, str_hex(data.chk.ptr, data.chk.len), str_ctrl(data.chk.ptr, data.chk.len)); if ((len == STR_SIZE(SPOE_MSG_ARG_HDRS)) && (memcmp(str, STR_ADDRSIZE(SPOE_MSG_ARG_HDRS)) == 0)) { - if (_nNULL(mir->hdrs)) { + if (!LIST_ISEMPTY(&(mir->hdrs))) { f_log(frame, _E("arg[%d] '%.*s': Duplicated argument"), i, (int)len, str); retval = FUNC_RET_ERROR; } - else if (_NULL(mir->hdrs = spoa_msg_arg_hdrs(frame, data.chk.ptr, data.chk.ptr + data.chk.len - 1))) - retval = FUNC_RET_ERROR; + else + retval = spoa_msg_arg_hdrs(frame, data.chk.ptr, data.chk.ptr + data.chk.len - 1, &(mir->hdrs)); } else if ((len == STR_SIZE(SPOE_MSG_ARG_BODY)) && (memcmp(str, STR_ADDRSIZE(SPOE_MSG_ARG_BODY)) == 0)) { if (_NULL(spoa_msg_arg_dup(frame, i, str, len, &data, &(mir->body), &(mir->body_size), "allocate memory for body"))) @@ -424,7 +422,7 @@ int spoa_msg_mirror(struct spoe_frame *frame, const char **buf, const char *end) f_log(frame, _E("HTTP request method not set")); else if (_NULL(mir->version)) f_log(frame, _E("HTTP version not set")); - else if (_NULL(mir->hdrs)) + else if (LIST_ISEMPTY(&(mir->hdrs))) f_log(frame, _E("HTTP headers not set")); else if (_NULL(mir->url = malloc(url_len))) f_log(frame, _E("Failed to allocate memory")); @@ -488,7 +486,7 @@ void mir_ptr_free(struct mirror **data) if (_NULL(data) || _NULL(*data)) return; - W_DBG(NOTICE, NULL, " freeing mirror { \"%s\" \"%s\" \"%s\" %d \"%s\" %p %p %zu/%zu }", (*data)->url, (*data)->path, (*data)->method, (*data)->request_method, (*data)->version, (*data)->hdrs, (*data)->body, (*data)->body_head, (*data)->body_size); + W_DBG(NOTICE, NULL, " freeing mirror { \"%s\" \"%s\" \"%s\" %d \"%s\" { %p %p } %p %zu/%zu }", (*data)->url, (*data)->path, (*data)->method, (*data)->request_method, (*data)->version, (*data)->hdrs.p, (*data)->hdrs.n, (*data)->body, (*data)->body_head, (*data)->body_size); PTR_FREE((*data)->out_address); PTR_FREE((*data)->url); @@ -496,10 +494,11 @@ void mir_ptr_free(struct mirror **data) PTR_FREE((*data)->method); PTR_FREE((*data)->version); - list_for_each_entry_safe(hdr, hdr_back, (*data)->hdrs, list) + list_for_each_entry_safe(hdr, hdr_back, &((*data)->hdrs), list) { + LIST_DEL(&(hdr->list)); buffer_ptr_free(&hdr); + } - PTR_FREE((*data)->hdrs); PTR_FREE((*data)->body); PTR_FREE(*data); }