From e02c597dbd0d667083e7afb4af3700741b074fc5 Mon Sep 17 00:00:00 2001 From: Utkarash Kumar Singh Date: Mon, 28 Sep 2026 18:13:36 +0100 Subject: [PATCH] Revert "fix: reject request headers that contain CR or LF (#299)" This reverts commit 62892ab8cbf25f5ed9ba9dddd02b3823bf2b7750. --- README.md | 2 - src/core.c | 30 +-------- test/test_http_header_crlf.py | 120 ---------------------------------- 3 files changed, 3 insertions(+), 149 deletions(-) delete mode 100644 test/test_http_header_crlf.py diff --git a/README.md b/README.md index abd1ee8..128946e 100644 --- a/README.md +++ b/README.md @@ -90,8 +90,6 @@ When any of the three request functions (`http_get`, `http_post`, `http_delete`) Once a response is received, it gets stored in the `_http_response` table. By monitoring this table, you can keep track of response statuses and messages. -A request whose headers contain a carriage return or line feed is not sent. It gets an `ERROR` response naming the header, since libcurl terminates headers with CRLF and an embedded one would let the header smuggle extra headers or a body into the request. - > [!IMPORTANT] > Inserting directly into `net.http_request_queue` won't wake the worker, you must use the request functions. Rows inserted directly are only processed the next time the worker wakes up. > We do it this way to avoid polling the `net.http_request_queue` table, which would pollute `pg_stat_statements` and cause unnecessary activity from the worker. diff --git a/src/core.c b/src/core.c index f0fc1d2..314e1d1 100644 --- a/src/core.c +++ b/src/core.c @@ -22,21 +22,7 @@ static size_t body_cb(void *contents, size_t size, size_t nmemb, void *userp) { return realsize; } -// A header with a CR or LF in it can inject extra headers or a body into the request, since libcurl -// ends every header with CRLF. Such requests are not sent. The message only names the header, the -// value may hold credentials. -static char *crlf_header_rejection(const char *hdr) { - size_t bad = strcspn(hdr, "\r\n"); - if (hdr[bad] == '\0') return NULL; - - size_t name_len = strcspn(hdr, ":"); - if (name_len > bad) name_len = bad; - - return psprintf("header \"%.*s\" contains a carriage return or line feed", (int)name_len, hdr); -} - -static struct curl_slist *pg_text_array_to_slist(ArrayType *array, struct curl_slist *headers, - char **rejected_reason) { +static struct curl_slist *pg_text_array_to_slist(ArrayType *array, struct curl_slist *headers) { ArrayIterator iterator; Datum value; bool isnull; @@ -50,17 +36,7 @@ static struct curl_slist *pg_text_array_to_slist(ArrayType *array, struct curl_s } hdr = TextDatumGetCString(value); - - char *reason = crlf_header_rejection(hdr); - if (reason) { - if (*rejected_reason == NULL) - *rejected_reason = reason; - else - pfree(reason); - } else { - EREPORT_CURL_SLIST_APPEND(headers, hdr); - } - + EREPORT_CURL_SLIST_APPEND(headers, hdr); pfree(hdr); } array_free_iterator(iterator); @@ -91,7 +67,7 @@ void init_curl_handle(CurlHandle *handle, RequestQueueRow row) { ArrayType *pgHeaders = DatumGetArrayTypeP(row.headersBin.value); struct curl_slist *request_headers = NULL; - request_headers = pg_text_array_to_slist(pgHeaders, request_headers, &handle->rejected_reason); + request_headers = pg_text_array_to_slist(pgHeaders, request_headers); EREPORT_CURL_SLIST_APPEND(request_headers, "User-Agent: pg_net/" EXTVERSION); diff --git a/test/test_http_header_crlf.py b/test/test_http_header_crlf.py deleted file mode 100644 index ceae784..0000000 --- a/test/test_http_header_crlf.py +++ /dev/null @@ -1,120 +0,0 @@ -import pytest -from common import pg_collect_response, pg_http_request -from test_http_timeout import wait_for_responses - - -@pytest.mark.parametrize( - "header, reported_name", - [ - ('{"X-Test": "value\\n"}', "X-Test"), - ('{"X-Test": "value\\r"}', "X-Test"), - ('{"X-Test": "value\\r\\nInjected: yes"}', "X-Test"), - ('{"X-Te\\nst": "value"}', "X-Te"), - ('{"\\r\\nInjected": "yes"}', ""), - ], -) -def test_headers_with_cr_or_lf_are_rejected(conn, header, reported_name): - """A header containing CR or LF is not sent and gets an ERROR response naming the header""" - - request_id = pg_http_request( - conn, - "select net.http_get(url := 'http://localhost:8080/headers', headers := %s::jsonb)", - (header,), - ) - - (status_code, error_msg, timed_out) = wait_for_responses(conn, [request_id])[ - request_id - ] - - assert status_code is None - assert timed_out is False - assert ( - error_msg == f'header "{reported_name}" contains a carriage return or line feed' - ) - assert "value" not in error_msg - assert "Injected" not in error_msg - - -def test_headers_without_cr_or_lf_are_sent(conn): - """Ordinary headers still reach the server""" - - request_id = pg_http_request( - conn, - """select net.http_get( - url := 'http://localhost:8080/headers', - headers := '{"X-Test": "plain value", "accept": "application/json"}'::jsonb - )""", - ) - - response = pg_collect_response(conn, request_id) - - assert response["status"] == "SUCCESS" - assert "X-Test" in response["body"] - - -def test_rejected_header_does_not_affect_the_rest_of_the_batch(conn): - """One bad header in a batch gets an ERROR row, the other requests are sent normally""" - - bad = pg_http_request( - conn, - """select net.http_get(url := 'http://localhost:8080/headers', headers := '{"X-Test": "a\\nb"}'::jsonb)""", - ) - get = pg_http_request( - conn, "select net.http_get(url := 'http://localhost:8080/headers')" - ) - post = pg_http_request( - conn, - "select net.http_post(url := 'http://localhost:8080/anything', body := '{}'::jsonb)", - ) - - responses = wait_for_responses(conn, [bad, get, post]) - - assert responses[bad][0] is None - assert ( - responses[bad][1] == 'header "X-Test" contains a carriage return or line feed' - ) - assert responses[get][0] == 200 - assert responses[post][0] == 200 - - -def test_direct_insert_with_cr_or_lf_header_is_rejected(conn): - """Rows inserted straight into the queue go through the same check""" - - request_id = pg_http_request( - conn, - """insert into net.http_request_queue(method, url, headers, timeout_milliseconds) - values ('GET', 'http://localhost:8080/headers', '{"X-Test": "a\\rb"}'::jsonb, 5000) - returning id""", - ) - conn.execute("select net.wake()") - conn.commit() - - (status_code, error_msg, _) = wait_for_responses(conn, [request_id])[request_id] - - assert status_code is None - assert error_msg == 'header "X-Test" contains a carriage return or line feed' - - follow_up = pg_http_request( - conn, "select net.http_get(url := 'http://localhost:8080/headers')" - ) - assert wait_for_responses(conn, [follow_up])[follow_up][0] == 200 - - -def test_timeout_rejection_is_reported_before_the_header_one(conn): - """A request with both a bad timeout and a bad header gets the timeout message""" - - request_id = pg_http_request( - conn, - """select net.http_get( - url := 'http://localhost:8080/headers', - headers := '{"X-Test": "a\\nb"}'::jsonb, - timeout_milliseconds := 0 - )""", - ) - - (_, error_msg, _) = wait_for_responses(conn, [request_id])[request_id] - - assert ( - error_msg - == "timeout_milliseconds must be between 1 and 600000 (pg_net.max_timeout_ms), got 0" - )