Fix case where POST for 301/302 caused an SEGFAULT and add a test

UkuLoskit · Feb 17, 2024 · 4568765 · 4568765
1 parent b58eb61
commit 4568765
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 10 deletions.
diff --git 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
@@ -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: ""
+