This is an automated email from the ASF dual-hosted git repository.
yuqi1129 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new 654106b5ca [#13735] fix(client-python): isolate per-request headers
(#13743)
654106b5ca is described below
commit 654106b5cac5a8671066aa0956f0899c518e4dd6
Author: Thunderkeg <[email protected]>
AuthorDate: Fri Oct 16 02:45:28 2026 -0700
[#13735] fix(client-python): isolate per-request headers (#13743)
### What changes were proposed in this pull request?
Make HTTP request headers local to each request. The client now copies
configured headers and merges per-request headers into a request-local
map, preserving the existing override order and JSON/form content-type
behavior. Add unit and Fileset integration coverage for header isolation
after successful and failed requests.
### Why are the changes needed?
Per-request headers were being merged into the HTTP client's persistent
headers, so they could leak into later calls and mutate the
caller-provided header dictionary. This change keeps request-specific
context, including CallerContext headers, scoped to the request that
supplied it.
Fix: #13735
### Does this PR introduce _any_ user-facing change?
Yes. It fixes per-request headers so they no longer affect subsequent
calls made through the same client. There are no API additions or
removals.
### How was this patch tested?
- From `clients/client-python/tests/unittests`, `python -m coverage run
--branch -m unittest`: 1,183 tests passed under Python 3.12.12.
- From `clients/client-python/tests/integration`,
`START_EXTERNAL_GRAVITINO=true PROJECT_VERSION=2.0.0-SNAPSHOT python -m
unittest -v
test_fileset_catalog.TestFilesetCatalog.test_get_file_location`: 1 test
passed against the matching Docker server.
- `ruff check`, `ruff format --check`, and `git diff --check` passed.
- `./gradlew :clients:client-python:compileDistribution -PskipWeb=true
-x test` passed with Corretto 17.0.18.
Signed-off-by: jiangxt2 <[email protected]>
---
.../client-python/gravitino/utils/http_client.py | 18 +--
.../tests/integration/test_fileset_catalog.py | 26 ++++
.../tests/unittests/test_http_client_headers.py | 153 +++++++++++++++++++++
3 files changed, 189 insertions(+), 8 deletions(-)
diff --git a/clients/client-python/gravitino/utils/http_client.py
b/clients/client-python/gravitino/utils/http_client.py
index 1196e17ef9..a0caada82d 100644
--- a/clients/client-python/gravitino/utils/http_client.py
+++ b/clients/client-python/gravitino/utils/http_client.py
@@ -93,7 +93,7 @@ class HTTPClient:
client_config
)
self.host = host
- self.request_headers = request_headers or {}
+ self.request_headers = dict(request_headers or {})
self.timeout = gravitino_client_config.get_client_request_timeout()
self.is_debug = is_debug
self.auth_data_provider = auth_data_provider
@@ -113,7 +113,9 @@ class HTTPClient:
return url
def _update_headers(self, request_headers):
- self.request_headers.update(request_headers)
+ merged_headers = self.request_headers.copy()
+ merged_headers.update(request_headers)
+ return merged_headers
def _mask_auth_headers(self, headers):
if self.is_debug:
@@ -173,22 +175,22 @@ class HTTPClient:
self._update_headers(self.JSON_HEADER)
elif data:
request_data = urlencode(data.to_dict()).encode()
- self._update_headers(self.FORMDATA_HEADER)
+ request_headers = self._update_headers(self.FORMDATA_HEADER)
else:
if json:
request_data = json.to_json().encode("utf-8")
- self._update_headers(self.JSON_HEADER)
+ request_headers = self._update_headers(self.JSON_HEADER)
if headers:
- self._update_headers(headers)
+ request_headers.update(headers)
opener = build_opener()
request = Request(self._build_url(endpoint, params), data=request_data)
- if self.request_headers:
- for key, value in self.request_headers.items():
+ if request_headers:
+ for key, value in request_headers.items():
request.add_header(key, value)
- if request_data and ("Content-Type" not in self.request_headers):
+ if request_data and ("Content-Type" not in request_headers):
request.add_header("Content-Type", "application/json")
if self.auth_data_provider is not None:
request.add_header(
diff --git a/clients/client-python/tests/integration/test_fileset_catalog.py
b/clients/client-python/tests/integration/test_fileset_catalog.py
index 41e7fb35e5..b5f959d24d 100644
--- a/clients/client-python/tests/integration/test_fileset_catalog.py
+++ b/clients/client-python/tests/integration/test_fileset_catalog.py
@@ -350,12 +350,28 @@ class TestFilesetCatalog(IntegrationTestEnv):
"location2": "/tmp/test_get_file_location2",
}
self.create_custom_fileset(fileset_ident, None, locations, "location1")
+ caller_context = CallerContext(
+ {
+ FilesetAuditConstants.HTTP_HEADER_FILESET_DATA_OPERATION:
FilesetDataOperation.GET_FILE_STATUS.name
+ }
+ )
+ CallerContextHolder.set(caller_context)
actual_file_location = (
self.gravitino_client.load_catalog(name=self.catalog_name)
.as_fileset_catalog()
.get_file_location(fileset_ident, "/test/test.txt")
)
+ self.assertEqual(
+ actual_file_location,
f"file:{locations['location1']}/test/test.txt"
+ )
+ self.assertIsNone(CallerContextHolder.get())
+
+ actual_file_location = (
+ self.gravitino_client.load_catalog(name=self.catalog_name)
+ .as_fileset_catalog()
+ .get_file_location(fileset_ident, "/test/test.txt")
+ )
self.assertEqual(
actual_file_location,
f"file:{locations['location1']}/test/test.txt"
)
@@ -373,3 +389,13 @@ class TestFilesetCatalog(IntegrationTestEnv):
.as_fileset_catalog()
.get_file_location(fileset_ident, "")
)
+
+ self.assertIsNone(CallerContextHolder.get())
+ actual_file_location = (
+ self.gravitino_client.load_catalog(name=self.catalog_name)
+ .as_fileset_catalog()
+ .get_file_location(fileset_ident, "/test/test.txt")
+ )
+ self.assertEqual(
+ actual_file_location,
f"file:{locations['location1']}/test/test.txt"
+ )
diff --git a/clients/client-python/tests/unittests/test_http_client_headers.py
b/clients/client-python/tests/unittests/test_http_client_headers.py
new file mode 100644
index 0000000000..e5ace7862a
--- /dev/null
+++ b/clients/client-python/tests/unittests/test_http_client_headers.py
@@ -0,0 +1,153 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import unittest
+from io import BytesIO
+from urllib.error import HTTPError
+from unittest.mock import Mock, patch
+
+from gravitino.exceptions.base import RESTException
+from gravitino.exceptions.handlers.rest_error_handler import REST_ERROR_HANDLER
+from gravitino.utils.http_client import HTTPClient
+
+
+class TestHTTPClientHeaders(unittest.TestCase):
+ @staticmethod
+ def _success_response():
+ response = Mock()
+ response.getcode.return_value = 200
+ response.read.return_value = b"{}"
+ response.info.return_value = {}
+ response.url = "http://localhost:8090/test";
+ return response
+
+ @staticmethod
+ def _capture_requests(mocked_build_opener, responses):
+ requests = []
+ response_iterator = iter(responses)
+
+ def open_request(request, timeout):
+ requests.append(request)
+ response = next(response_iterator)
+ if isinstance(response, Exception):
+ raise response
+ return response
+
+ mocked_build_opener.return_value.open.side_effect = open_request
+ return requests
+
+ @staticmethod
+ def _header_value(request, header_name):
+ for name, value in request.header_items():
+ if name.lower() == header_name.lower():
+ return value
+ return None
+
+ def test_request_headers_do_not_leak_between_calls(self):
+ configured_headers = {
+ "X-Configured": "configured",
+ "X-Override": "configured",
+ }
+ client = HTTPClient("http://localhost:8090";,
request_headers=configured_headers)
+
+ with patch("gravitino.utils.http_client.build_opener") as build_opener:
+ requests = self._capture_requests(
+ build_opener, [self._success_response(),
self._success_response()]
+ )
+ client.get(
+ "/first",
+ headers={"X-Per-Request": "first", "X-Override": "request"},
+ )
+ client.get("/second")
+
+ self.assertEqual("configured", self._header_value(requests[0],
"X-Configured"))
+ self.assertEqual("request", self._header_value(requests[0],
"X-Override"))
+ self.assertEqual("first", self._header_value(requests[0],
"X-Per-Request"))
+ self.assertEqual("configured", self._header_value(requests[1],
"X-Configured"))
+ self.assertEqual("configured", self._header_value(requests[1],
"X-Override"))
+ self.assertIsNone(self._header_value(requests[1], "X-Per-Request"))
+ self.assertEqual(
+ {"X-Configured": "configured", "X-Override": "configured"},
+ configured_headers,
+ )
+ self.assertEqual(configured_headers, client.request_headers)
+
+ def test_json_and_form_headers_are_request_local(self):
+ configured_headers = {
+ "X-Configured": "configured",
+ "Content-Type": "configured/type",
+ }
+ client = HTTPClient("http://localhost:8090";,
request_headers=configured_headers)
+ form_data = Mock()
+ form_data.to_dict.return_value = {"key": "value"}
+ json_data = Mock()
+ json_data.to_json.return_value = '{"key":"value"}'
+
+ with patch("gravitino.utils.http_client.build_opener") as build_opener:
+ requests = self._capture_requests(
+ build_opener, [self._success_response(),
self._success_response()]
+ )
+ client.post("/form", data=form_data)
+ client.post("/json", json=json_data)
+
+ self.assertEqual(
+ "application/x-www-form-urlencoded",
+ self._header_value(requests[0], "Content-Type"),
+ )
+ self.assertEqual(
+ "application/vnd.gravitino.v1+json",
+ self._header_value(requests[0], "Accept"),
+ )
+ self.assertEqual(
+ "application/json", self._header_value(requests[1], "Content-Type")
+ )
+ self.assertEqual(
+ "application/vnd.gravitino.v1+json",
+ self._header_value(requests[1], "Accept"),
+ )
+ for request in requests:
+ self.assertEqual("configured", self._header_value(request,
"X-Configured"))
+ self.assertEqual("configured/type", configured_headers["Content-Type"])
+ self.assertEqual(configured_headers, client.request_headers)
+
+ def test_failed_request_headers_do_not_leak(self):
+ client = HTTPClient("http://localhost:8090";)
+ error_body = (
+ b'{"code":9999,"type":"RESTException",'
+ b'"message":"request failed","stack":null}'
+ )
+ error = HTTPError(
+ "http://localhost:8090/failure";,
+ 500,
+ "Internal Server Error",
+ None,
+ BytesIO(error_body),
+ )
+
+ with patch("gravitino.utils.http_client.build_opener") as build_opener:
+ requests = self._capture_requests(
+ build_opener, [error, self._success_response()]
+ )
+ with self.assertRaises(RESTException):
+ client.get(
+ "/failure",
+ headers={"X-Per-Request": "failed"},
+ error_handler=REST_ERROR_HANDLER,
+ )
+ client.get("/after-failure")
+
+ self.assertEqual("failed", self._header_value(requests[0],
"X-Per-Request"))
+ self.assertIsNone(self._header_value(requests[1], "X-Per-Request"))
