From 4568765ce4fb759a3c1705865784b36f1855d755 Mon Sep 17 00:00:00 2001 From: Uku Loskit Date: Sat, 17 Feb 2024 13:36:03 +0200 Subject: [PATCH] Fix case where POST for 301/302 caused an SEGFAULT and add a test --- extensions/omni_httpc/omni_httpc.c | 31 +++++++++++++++------- extensions/omni_httpc/tests/redirect.yml | 33 ++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 10 deletions(-) diff --git a/extensions/omni_httpc/omni_httpc.c b/extensions/omni_httpc/omni_httpc.c index 100f8a385..2de3205d5 100644 --- a/extensions/omni_httpc/omni_httpc.c +++ b/extensions/omni_httpc/omni_httpc.c @@ -298,17 +298,21 @@ static h2o_httpclient_body_cb on_head(h2o_httpclient_t *client, const char *errs // Handle Redirect if necessary if (handle_redirect) { - h2o_headers_t *headers_vec = (h2o_headers_t *)palloc0(sizeof(*headers_vec)); - headers_vec->entries = args->headers; - headers_vec->size = args->num_headers; - headers_vec->capacity = args->num_headers; + h2o_headers_t *response_headers_vec = (h2o_headers_t *)palloc0(sizeof(*response_headers_vec)); + response_headers_vec->entries = args->headers; + response_headers_vec->size = args->num_headers; + response_headers_vec->capacity = args->num_headers; + ssize_t location_index; - if ((location_index = h2o_find_header(headers_vec, H2O_TOKEN_LOCATION, -1)) > -1) { + if ((location_index = h2o_find_header(response_headers_vec, H2O_TOKEN_LOCATION, -1)) > -1) { // create a new request based on the original request struct request *new_req = palloc0(sizeof(struct request)); new_req->complete = false; new_req->connected = false; new_req->headers = req->headers; + h2o_headers_t *request_headers_vec = (h2o_headers_t *)palloc0(sizeof(*request_headers_vec)); + request_headers_vec->entries = req->headers; + request_headers_vec->size = req->num_headers; initStringInfo(&new_req->body); appendStringInfoSpaces(&new_req->body, sizeof(struct varlena)); @@ -323,23 +327,30 @@ static h2o_httpclient_body_cb on_head(h2o_httpclient_t *client, const char *errs new_req->method = req->method; new_req->request_body = req->request_body; } else { - // omit the request body - // we don't need to remove the header, as it will be automatically omitted if the body - // is empty + // omit the request body and content-length header + int content_length_index = + h2o_find_header(request_headers_vec, H2O_TOKEN_CONTENT_LENGTH, -1); + if (content_length_index != -1) { + ereport(WARNING, errmsg("deleting content-length header")); + h2o_delete_header(request_headers_vec, content_length_index); + } new_req->request_body = h2o_iovec_init(NULL, 0); // if the method is POST, override the method as GET if (h2o_memis(req->method.base, req->method.len, H2O_STRLIT("POST"))) { + ereport(WARNING, errmsg("following a redirect from a POST request to a non-POST URL")); new_req->method = h2o_iovec_init(H2O_STRLIT("GET")); } else { new_req->method = req->method; } } + new_req->headers = request_headers_vec->entries; + new_req->num_headers = request_headers_vec->size; h2o_url_t url; // re-evaluate location index after the headers have been modified - location_index = h2o_find_header(headers_vec, H2O_TOKEN_LOCATION, -1); + location_index = h2o_find_header(response_headers_vec, H2O_TOKEN_LOCATION, -1); - h2o_iovec_t location_header = headers_vec->entries[location_index].value; + h2o_iovec_t location_header = response_headers_vec->entries[location_index].value; // try to parse the URL as an absolute URL int result = h2o_url_parse(client->pool, location_header.base, location_header.len, &url); if (result == -1) { diff --git a/extensions/omni_httpc/tests/redirect.yml b/extensions/omni_httpc/tests/redirect.yml index 41a040e10..fa7a302cc 100644 --- a/extensions/omni_httpc/tests/redirect.yml +++ b/extensions/omni_httpc/tests/redirect.yml @@ -262,3 +262,36 @@ tests: headers: null body: "test" + - name: POST requests should be converted to GET for HTTP 301/302 redirects with the body omitted + query: | + with response as (select * + from omni_httpc.http_execute_with_options( + omni_httpc.http_execute_options(follow_redirects => true), + omni_httpc.http_request( + 'http://127.0.0.1:' || + (select effective_port from omni_httpd.listeners) || + '/test?q=absolute_url&status=302', + method => 'POST', + body => 'test' + ) + ) + ) + select response.status, + (select json_agg(json_build_object(h.name, h.value)) + from + unnest(response.headers) h + where + h.name not in ('server', 'content-length', 'connection')) as headers, + convert_from(response.body, 'utf-8')::json as body + from response + results: + - status: 200 + headers: + - content-type: application/json + body: + method: GET + path: /test + qs: q=destination + headers: null + body: "" +