From 80fd0844e2ad11f15b4d082c7510d2efe166b830 Mon Sep 17 00:00:00 2001 From: Dmitrii Iutkin Date: Thu, 29 Oct 2026 16:18:05 +0200 Subject: [PATCH 01/10] Do not follow HTTP redirects in the API and inference clients InferenceClient, HTTPClient and the authentication token requests now send requests with allow_redirects=False. A 3xx response raises InferenceClientError or APIException. AsyncInferenceExecution.status_json() and result() now use the client timeout and raise on an HTTP error status. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 9 + CLAUDE.md | 2 + .../authentication/test_authentication.py | 33 +++ .../http_client/test_http_client.py | 67 ++++- .../inference_client/test_inference_client.py | 237 +++++++++++++++++- verda/authentication/_authentication.py | 8 +- verda/http_client/_http_client.py | 27 +- verda/inference_client/_inference_client.py | 89 +++++-- 8 files changed, 451 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index de0e581..e96944a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Security + +- `InferenceClient`, `HTTPClient` and the authentication token requests no longer follow HTTP redirects. A 3xx response now raises `InferenceClientError` or `APIException`. The error message includes the `Location` value. + +### Changed + +- `AsyncInferenceExecution.status_json()` and `result()` now use the client `timeout_seconds` and raise `InferenceClientError` on an HTTP error status. +- `HTTPClient` methods ignore the `allow_redirects` keyword argument. + ## [1.25.0] - 2026-10-23 ### Deprecated diff --git a/CLAUDE.md b/CLAUDE.md index bbabe2d..3b9da6e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -105,6 +105,8 @@ Reject, do not encode: `requests` decodes `%2E` back to `.` before sending, and All five verbs delegate to a single private `_request`, which is where the url is built and validated. Add new verbs by delegating to it, never by calling `requests` directly. +**Never follow a redirect.** Every request that carries credentials is sent with `allow_redirects=False`, and a 3xx raises: `handle_error` for `HTTPClient` and the auth token calls, and `InferenceClient._send` for inference. `requests` keeps `Authorization` on a same-host redirect, and a 307/308 also keeps the method and body. The `Location` target never went through path validation. Any new send site must pass `allow_redirects=False` and go through the existing error check. Inference requests, including `AsyncInferenceExecution` status and result, go through `InferenceClient._send`, never `_session` directly. + The check is `_encode_path_segment` in `verda/http_client/_http_client.py`; `path_params` is the only supported way to put a caller-supplied value into a path. `_add_base_url` re-asserts the same allowlist on the finished path, as a backstop for a call site that skips `path_params`. `verda.helpers.has_relative_path_segment` strips the query string and decodes escapes and encoded separators. It is for `InferenceClient` only, whose `path` spans several segments and may carry a query string. Do not use it in the http client, and do not re-implement it. diff --git a/tests/unit_tests/authentication/test_authentication.py b/tests/unit_tests/authentication/test_authentication.py index 2d50d86..b9ebb0b 100644 --- a/tests/unit_tests/authentication/test_authentication.py +++ b/tests/unit_tests/authentication/test_authentication.py @@ -242,3 +242,36 @@ def test_is_expired(self, authentication_service): # assert assert is_expired_current assert not is_expired_future + + @pytest.mark.parametrize('status', [302, 307, 308]) + def test_authenticate_redirect_failed(self, authentication_service, endpoint, status): + # arrange + target = BASE_URL + '/moved' + responses.add(responses.POST, endpoint, status=status, headers={'Location': target}) + responses.add(responses.POST, target, json={}) + responses.add(responses.GET, target, json={}) + + # act + with pytest.raises(APIException) as excinfo: + authentication_service.authenticate() + + # assert + assert f'unexpected redirect ({status})' in excinfo.value.message + assert len(responses.calls) == 1 + + @pytest.mark.parametrize('status', [302, 307, 308]) + def test_refresh_redirect_failed(self, authentication_service, endpoint, status): + # arrange + authentication_service._refresh_token = REFRESH_TOKEN + target = BASE_URL + '/moved' + responses.add(responses.POST, endpoint, status=status, headers={'Location': target}) + responses.add(responses.POST, target, json={}) + responses.add(responses.GET, target, json={}) + + # act + with pytest.raises(APIException) as excinfo: + authentication_service.refresh() + + # assert + assert f'unexpected redirect ({status})' in excinfo.value.message + assert len(responses.calls) == 1 diff --git a/tests/unit_tests/http_client/test_http_client.py b/tests/unit_tests/http_client/test_http_client.py index cb80681..2ecd487 100644 --- a/tests/unit_tests/http_client/test_http_client.py +++ b/tests/unit_tests/http_client/test_http_client.py @@ -17,10 +17,11 @@ from unittest.mock import Mock import pytest +import requests import responses # https://github.com/getsentry/responses from verda.exceptions import APIException -from verda.http_client._http_client import _encode_path_segment +from verda.http_client._http_client import _encode_path_segment, handle_error INVALID_REQUEST = 'invalid_request' INVALID_REQUEST_MESSAGE = 'Your existence is invalid' @@ -497,3 +498,67 @@ def test_delete_failed(self, http_client): # assert assert excinfo.value.code == INVALID_REQUEST assert excinfo.value.message == INVALID_REQUEST_MESSAGE + + @pytest.mark.parametrize('status', [301, 302, 303, 307, 308]) + @pytest.mark.parametrize('method', ['get', 'post', 'put', 'patch', 'delete']) + def test_request_methods_redirect_failed(self, http_client, method, status): + # arrange + url = http_client._base_url + '/test' + target = http_client._base_url + '/moved' + responses.add(method.upper(), url, status=status, headers={'Location': target}) + responses.add(method.upper(), target, json={}) + responses.add(responses.GET, target, json={}) + + # act + with pytest.raises(APIException) as excinfo: + getattr(http_client, method)('/test') + + # assert + assert excinfo.value.code is None + assert f'unexpected redirect ({status})' in excinfo.value.message + assert len(responses.calls) == 1 + + @pytest.mark.parametrize('method', ['get', 'post', 'put', 'patch', 'delete']) + def test_request_methods_accept_allow_redirects_false(self, http_client, method): + # arrange + url = http_client._base_url + '/test' + responses.add(method.upper(), url, json={}) + + # act + response = getattr(http_client, method)('/test', allow_redirects=False) + + # assert + assert response.status_code == 200 + assert len(responses.calls) == 1 + + @pytest.mark.parametrize('method', ['get', 'post', 'put', 'patch', 'delete']) + def test_request_methods_allow_redirects_true_redirect_failed(self, http_client, method): + # arrange + url = http_client._base_url + '/test' + target = http_client._base_url + '/moved' + responses.add(method.upper(), url, status=307, headers={'Location': target}) + responses.add(method.upper(), target, json={}) + + # act + with pytest.raises(APIException) as excinfo: + getattr(http_client, method)('/test', allow_redirects=True) + + # assert + assert 'unexpected redirect (307)' in excinfo.value.message + assert len(responses.calls) == 1 + + +def test_handle_error_redirect_failed(): + # arrange + response = requests.Response() + response.status_code = 302 + response.headers['Location'] = 'https://api.example.com/v1/moved' + + # act + with pytest.raises(APIException) as excinfo: + handle_error(response) + + # assert + assert excinfo.value.message == ( + "unexpected redirect (302) to 'https://api.example.com/v1/moved'; redirects are not followed" + ) diff --git a/tests/unit_tests/inference_client/test_inference_client.py b/tests/unit_tests/inference_client/test_inference_client.py index dd24e13..da8adc9 100644 --- a/tests/unit_tests/inference_client/test_inference_client.py +++ b/tests/unit_tests/inference_client/test_inference_client.py @@ -13,8 +13,16 @@ # limitations under the License. import pytest +import requests +import responses # https://github.com/getsentry/responses +from responses import matchers -from verda.inference_client import InferenceClient, InferenceClientError +from verda.inference_client import ( + AsyncInferenceExecution, + AsyncStatus, + InferenceClient, + InferenceClientError, +) BASE_URL = 'https://inference.example.com/v1/my-deployment' @@ -102,3 +110,230 @@ def test_base_url_without_a_deployment_path_is_rejected(self, base_url): def test_the_deployment_name_is_sliced_out_correctly(self, inference_client): assert inference_client.deployment_name == 'my-deployment' assert inference_client.base_domain == 'https://inference.example.com/v1' + + +PREDICT_URL = f'{BASE_URL}/predict' +STATUS_URL = 'https://inference.example.com/v1/status/my-deployment' +RESULT_URL = 'https://inference.example.com/v1/result/my-deployment' +REDIRECT_URL = 'https://inference.example.com/v1/moved' +EXECUTION_ID = 'exec-1' + +REDIRECT_STATUSES = [301, 302, 303, 307, 308] + + +def _execution(client): + return AsyncInferenceExecution(client, EXECUTION_ID, AsyncStatus.Initialized) + + +# (method, url the call sends to, call) +ENTRY_POINTS = { + 'run_sync': ('POST', PREDICT_URL, lambda c: c.run_sync({'a': 1}, path='predict')), + 'run': ('POST', PREDICT_URL, lambda c: c.run({'a': 1}, path='predict')), + 'run_no_response': ( + 'POST', + PREDICT_URL, + lambda c: c.run({'a': 1}, path='predict', no_response=True), + ), + 'get': ('GET', PREDICT_URL, lambda c: c.get('predict')), + 'post': ('POST', PREDICT_URL, lambda c: c.post('predict', json={'a': 1})), + 'put': ('PUT', PREDICT_URL, lambda c: c.put('predict', json={'a': 1})), + 'delete': ('DELETE', PREDICT_URL, lambda c: c.delete('predict')), + 'patch': ('PATCH', PREDICT_URL, lambda c: c.patch('predict', json={'a': 1})), + 'head': ('HEAD', PREDICT_URL, lambda c: c.head('predict')), + 'options': ('OPTIONS', PREDICT_URL, lambda c: c.options('predict')), + 'health': ('GET', f'{BASE_URL}/health', lambda c: c.health()), + 'status_json': ('GET', STATUS_URL, lambda c: _execution(c).status_json()), + 'result': ('GET', RESULT_URL, lambda c: _execution(c).result()), +} + + +class TestRedirectResponses: + """A 3xx response is reported to the caller as an error.""" + + @pytest.mark.parametrize('status', REDIRECT_STATUSES) + @pytest.mark.parametrize('entry_point', list(ENTRY_POINTS)) + def test_request_redirect_failed(self, inference_client, entry_point, status): + # arrange + method, url, call = ENTRY_POINTS[entry_point] + responses.add(method, url, status=status, headers={'Location': REDIRECT_URL}) + for target_method in ('GET', method): + responses.add(target_method, REDIRECT_URL, json={'Id': EXECUTION_ID}) + + # act + with pytest.raises(InferenceClientError, match=rf'redirect \({status}\)'): + call(inference_client) + + # assert + assert len(responses.calls) == 1 + assert responses.calls[0].request.url == url + + def test_request_redirect_to_another_host_failed(self, inference_client): + # arrange + inference_client.set_global_header('X-Request-Source', 'tests') + target = 'https://other.example.net/moved' + responses.add(responses.POST, PREDICT_URL, status=307, headers={'Location': target}) + responses.add(responses.POST, target, json={}) + + # act + with pytest.raises(InferenceClientError, match='redirects are not followed'): + inference_client.run_sync({'a': 1}, path='predict') + + # assert + assert len(responses.calls) == 1 + + def test_session_returns_a_redirect_as_is(self, inference_client): + # arrange + responses.add(responses.GET, PREDICT_URL, status=307, headers={'Location': REDIRECT_URL}) + responses.add(responses.GET, REDIRECT_URL, json={}) + + # act + response = inference_client._session.get(PREDICT_URL, allow_redirects=True) + + # assert + assert response.status_code == 307 + assert len(responses.calls) == 1 + + +class TestRequests: + """Successful inference requests.""" + + def test_run_sync_successful(self, inference_client): + # arrange + responses.add( + responses.POST, + PREDICT_URL, + json={'out': 1}, + match=[ + matchers.json_params_matcher({'a': 1}), + matchers.header_matcher({'Authorization': 'Bearer key-123', 'X-Extra': 'x'}), + ], + ) + + # act + response = inference_client.run_sync({'a': 1}, path='predict', headers={'X-Extra': 'x'}) + + # assert + assert response.status_code == 200 + assert response.output() == {'out': 1} + + def test_run_sync_stream_successful(self, inference_client): + # arrange + responses.add( + responses.POST, + PREDICT_URL, + body='line 1\nline 2\n', + content_type='text/event-stream', + ) + + # act + response = inference_client.run_sync({'a': 1}, path='predict', stream=True) + + # assert + assert list(response.stream()) == [b'line 1', b'line 2'] + + def test_run_then_status_and_result_successful(self, inference_client): + # arrange + responses.add( + responses.POST, + PREDICT_URL, + json={'Id': EXECUTION_ID}, + match=[matchers.header_matcher({'Prefer': 'respond-async'})], + ) + inference_id = matchers.header_matcher( + {'Authorization': 'Bearer key-123', 'X-Inference-Id': EXECUTION_ID} + ) + responses.add(responses.GET, STATUS_URL, json={'Status': 'Completed'}, match=[inference_id]) + responses.add(responses.GET, RESULT_URL, json={'out': 1}, match=[inference_id]) + + # act + execution = inference_client.run({'a': 1}, path='predict') + status = execution.status_json() + result = execution.result() + + # assert + assert execution.id == EXECUTION_ID + assert status == {'Status': 'Completed'} + assert execution.status() == AsyncStatus.Completed + assert result == {'out': 1} + + def test_result_text_successful(self, inference_client): + # arrange + responses.add(responses.GET, RESULT_URL, body='plain', content_type='text/plain') + + # act + result = _execution(inference_client).result() + + # assert + assert result == {'result': 'plain'} + + @pytest.mark.parametrize('entry_point', ['status_json', 'result']) + def test_async_requests_use_the_client_timeout(self, inference_client, entry_point): + # arrange + _, url, call = ENTRY_POINTS[entry_point] + responses.add(responses.GET, url, json={'Status': 'Completed'}) + + # act + call(inference_client) + + # assert + assert responses.calls[0].request.req_kwargs['timeout'] == inference_client.timeout_seconds + + @pytest.mark.parametrize('entry_point', ['status_json', 'result']) + def test_async_requests_failed(self, inference_client, entry_point): + # arrange + _, url, call = ENTRY_POINTS[entry_point] + responses.add(responses.GET, url, status=500, json={'error': 'boom'}) + + # act + with pytest.raises(InferenceClientError, match='500'): + call(inference_client) + + @pytest.mark.parametrize('entry_point', ['status_json', 'result']) + def test_async_requests_timed_out(self, inference_client, entry_point): + # arrange + _, url, call = ENTRY_POINTS[entry_point] + responses.add(responses.GET, url, body=requests.exceptions.ReadTimeout()) + + # act + with pytest.raises(InferenceClientError, match='timed out'): + call(inference_client) + + @pytest.mark.parametrize( + 'entry_point', ['get', 'post', 'put', 'delete', 'patch', 'head', 'options'] + ) + def test_http_method_helpers_successful(self, inference_client, entry_point): + # arrange + method, url, call = ENTRY_POINTS[entry_point] + responses.add(method, url, status=200) + + # act + response = call(inference_client) + + # assert + assert response.status_code == 200 + assert len(responses.calls) == 1 + + def test_get_with_params_successful(self, inference_client): + # arrange + responses.add( + responses.GET, + PREDICT_URL, + json={}, + match=[matchers.query_param_matcher({'q': 'x'})], + ) + + # act + response = inference_client.get('predict', params={'q': 'x'}) + + # assert + assert response.status_code == 200 + + def test_health_successful(self, inference_client): + # arrange + responses.add(responses.GET, f'{BASE_URL}/health', json={'ok': True}) + + # act + response = inference_client.health() + + # assert + assert response.json() == {'ok': True} diff --git a/verda/authentication/_authentication.py b/verda/authentication/_authentication.py index a88efb0..d23b78e 100644 --- a/verda/authentication/_authentication.py +++ b/verda/authentication/_authentication.py @@ -54,7 +54,9 @@ def authenticate(self) -> dict: 'client_secret': self._client_secret, } - response = requests.post(url, json=payload, headers=self._generate_headers()) + response = requests.post( + url, json=payload, headers=self._generate_headers(), allow_redirects=False + ) handle_error(response) auth_data = response.json() @@ -87,7 +89,9 @@ def refresh(self) -> dict: payload = {'grant_type': REFRESH_TOKEN, 'refresh_token': self._refresh_token} - response = requests.post(url, json=payload, headers=self._generate_headers()) + response = requests.post( + url, json=payload, headers=self._generate_headers(), allow_redirects=False + ) # if refresh token is also expired, authenticate again: if response.status_code == 401 or response.status_code == 400: diff --git a/verda/http_client/_http_client.py b/verda/http_client/_http_client.py index 4c9c71e..e393d5b 100644 --- a/verda/http_client/_http_client.py +++ b/verda/http_client/_http_client.py @@ -73,7 +73,10 @@ def _encode_path_segment(value: object) -> str: def handle_error(response: requests.Response) -> None: - """Checks for the response status code and raises an exception if it's 400 or higher. + """Checks for the response status code and raises an exception if it's 300 or higher. + + Requests are sent with ``allow_redirects=False``, so a 3xx arrives here unfollowed + and is an error: ``response.ok`` alone would return it to the caller as a success. Args: response: The API call response. @@ -81,6 +84,13 @@ def handle_error(response: requests.Response) -> None: Raises: APIException: An api exception with message and error type code. """ + if 300 <= response.status_code < 400: + location = response.headers.get('Location') + raise APIException( + None, + f'unexpected redirect ({response.status_code}) to {location!r}; ' + 'redirects are not followed', + ) if not response.ok: data = json.loads(response.text) code = data['code'] if 'code' in data else None @@ -294,6 +304,7 @@ def _request( params: Dictionary of querystring data to attach to the request. path_params: Values substituted into ``{name}`` placeholders in the url. **kwargs: Additional keyword arguments passed through to ``requests``. + ``allow_redirects`` is ignored: redirects are never followed. Returns: The response object. @@ -302,13 +313,25 @@ def _request( ValueError: If the url and ``path_params`` do not produce a safe path. APIException: An api exception with message and error type code. """ + # Accepted for compatibility but never passed on: True would follow a redirect, + # and False beside our own keyword would raise TypeError. + kwargs.pop('allow_redirects', None) + # Validate before the refresh so a rejected name costs no auth round-trip. url = self._add_base_url(self._build_path(url, path_params)) self._refresh_token_if_expired() + # A followed same-host redirect keeps the bearer token, method and body, and + # its target never went through the path validation above. response = requests.request( - method, url, json=json, headers=self._generate_headers(), params=params, **kwargs + method, + url, + json=json, + headers=self._generate_headers(), + params=params, + allow_redirects=False, + **kwargs, ) handle_error(response) diff --git a/verda/inference_client/_inference_client.py b/verda/inference_client/_inference_client.py index c458b15..876e24d 100644 --- a/verda/inference_client/_inference_client.py +++ b/verda/inference_client/_inference_client.py @@ -119,6 +119,18 @@ def stream(self, chunk_size: int = 512, as_text: bool = True) -> Generator[Any, yield chunk +class _NoRedirectSession(requests.Session): + """A session that never follows a redirect, even if asked to. + + Requests are sent with ``allow_redirects=False``. This is the backstop for a call + that forgets: the 3xx response is returned as-is instead of being followed. + """ + + def get_redirect_target(self, resp: requests.Response) -> None: # noqa: ARG002 + """Report no redirect target, so ``resolve_redirects`` has nothing to follow.""" + return None + + class InferenceClient: """Inference client.""" @@ -162,7 +174,7 @@ def __init__( self.base_domain = self.endpoint_base_url[: self.endpoint_base_url.rindex('/')] self.deployment_name = self.endpoint_base_url[self.endpoint_base_url.rindex('/') + 1 :] self.timeout_seconds = timeout_seconds - self._session = requests.Session() + self._session = _NoRedirectSession() self._global_headers = { 'Authorization': f'Bearer {inference_key}', 'Content-Type': 'application/json', @@ -265,14 +277,54 @@ def _make_request(self, method: str, path: str, **kwargs) -> requests.Response: InferenceClientError: If the request fails """ timeout = kwargs.pop('timeout_seconds', self.timeout_seconds) + return self._send( + method, + self._build_url(path), + path, + timeout, + headers=self._build_request_headers(kwargs.pop('headers', None)), + **kwargs, + ) + + def _send( + self, method: str, url: str, path: str, timeout: float | None, **kwargs + ) -> requests.Response: + """Send a request without following redirects. + + Every request carrying the inference key goes through here. A redirect is + not followed: ``requests`` keeps the ``Authorization`` header, method and body + on a same-host redirect, so a ``Location`` from the response would retarget + the request at a path that never went through ``_build_url``. + + Args: + method: HTTP method to use + url: Full request URL + path: The path as the caller gave it, for error messages + timeout: Request timeout in seconds + **kwargs: Additional arguments to pass to the request + + Returns: + Response object from the request + + Raises: + InferenceClientError: If the request fails or the response is a redirect + """ try: response = self._session.request( method=method, - url=self._build_url(path), - headers=self._build_request_headers(kwargs.pop('headers', None)), + url=url, timeout=timeout, + allow_redirects=False, **kwargs, ) + # raise_for_status() lets a 3xx through. + if 300 <= response.status_code < 400: + location = response.headers.get('Location') + response.close() + raise InferenceClientError( + f'Request to {path} returned a redirect ({response.status_code}) ' + f'to {location!r}; redirects are not followed' + ) response.raise_for_status() return response except requests.exceptions.Timeout as e: @@ -531,21 +583,30 @@ def status(self) -> AsyncStatus: """ return self._status + def _get(self, url: str) -> requests.Response: + """GET an async status or result url for this execution.""" + client = self._inference_client + return client._send( + 'GET', + url, + url, + client.timeout_seconds, + headers=client._build_request_headers({self.INFERENCE_ID_HEADER: self.id}), + ) + def status_json(self) -> dict[str, Any]: """Get the current status of the async inference execution. Return the status json. Returns: dict[str, Any]: The status response containing the execution status and other metadata + + Raises: + InferenceClientError: If the request fails """ url = ( f'{self._inference_client.base_domain}/status/{self._inference_client.deployment_name}' ) - response = self._inference_client._session.get( - url, - headers=self._inference_client._build_request_headers( - {self.INFERENCE_ID_HEADER: self.id} - ), - ) + response = self._get(url) response_json = response.json() self._status = AsyncStatus(response_json['Status']) @@ -557,16 +618,14 @@ def result(self) -> dict[str, Any]: Returns: dict[str, Any]: The results of the inference execution + + Raises: + InferenceClientError: If the request fails """ url = ( f'{self._inference_client.base_domain}/result/{self._inference_client.deployment_name}' ) - response = self._inference_client._session.get( - url, - headers=self._inference_client._build_request_headers( - {self.INFERENCE_ID_HEADER: self.id} - ), - ) + response = self._get(url) if response.headers['Content-Type'] == 'application/json': return response.json() From 4fdd94e72e47db1d4980f2c8d0e49d22a7586f2c Mon Sep 17 00:00:00 2001 From: Dmitrii Iutkin Date: Thu, 29 Oct 2026 17:13:16 +0200 Subject: [PATCH 02/10] Add opt-in follow_redirects to InferenceClient InferenceClient(follow_redirects=True) follows a redirect only if the target is inside the deployment: same scheme, host and port as endpoint_base_url, a path under the deployment path, and no relative path segment in the Location path. It follows at most 5 redirects. Any other redirect raises InferenceClientError. The default stays off. AsyncInferenceExecution status and result never follow a redirect. HTTPClient and the authentication calls do not change. When a redirect only adds or removes a trailing slash, the error message tells the caller to change path. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 5 + CLAUDE.md | 2 + .../inference_client/test_inference_client.py | 224 ++++++++++++++++++ verda/inference_client/_inference_client.py | 164 +++++++++++-- 4 files changed, 376 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e96944a..3d6c55e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- `InferenceClient` has a new `follow_redirects` argument. It is off by default. When it is on, the client follows a redirect only if the target is inside the deployment: same scheme, host and port, and a path under `endpoint_base_url`. The client follows at most 5 redirects. Any other redirect raises `InferenceClientError`. `AsyncInferenceExecution.status_json()` and `result()` never follow a redirect. +- When a redirect only adds or removes a trailing slash, the `InferenceClientError` message tells you to change `path`. + ### Security - `InferenceClient`, `HTTPClient` and the authentication token requests no longer follow HTTP redirects. A 3xx response now raises `InferenceClientError` or `APIException`. The error message includes the `Location` value. diff --git a/CLAUDE.md b/CLAUDE.md index 3b9da6e..c1ad730 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -107,6 +107,8 @@ All five verbs delegate to a single private `_request`, which is where the url i **Never follow a redirect.** Every request that carries credentials is sent with `allow_redirects=False`, and a 3xx raises: `handle_error` for `HTTPClient` and the auth token calls, and `InferenceClient._send` for inference. `requests` keeps `Authorization` on a same-host redirect, and a 307/308 also keeps the method and body. The `Location` target never went through path validation. Any new send site must pass `allow_redirects=False` and go through the existing error check. Inference requests, including `AsyncInferenceExecution` status and result, go through `InferenceClient._send`, never `_session` directly. +The one exception is `InferenceClient(follow_redirects=True)`, an opt-in for deployment calls only. `_send` then passes `allow_redirects=True`, and `_RedirectPolicySession.get_redirect_target` reports a target only if `_is_inside_deployment` accepts it: same scheme, host and port as `endpoint_base_url`, a path equal to the deployment path or under `/`, and a raw `Location` path that passes `has_relative_path_segment`. A rejected target stops `resolve_redirects`, and `_send` raises on the remaining 3xx. The session caps hops at 5. `AsyncInferenceExecution` status and result never follow, because those urls belong to the gateway. Do not add the flag to `HTTPClient` or the auth token calls. + The check is `_encode_path_segment` in `verda/http_client/_http_client.py`; `path_params` is the only supported way to put a caller-supplied value into a path. `_add_base_url` re-asserts the same allowlist on the finished path, as a backstop for a call site that skips `path_params`. `verda.helpers.has_relative_path_segment` strips the query string and decodes escapes and encoded separators. It is for `InferenceClient` only, whose `path` spans several segments and may carry a query string. Do not use it in the http client, and do not re-implement it. diff --git a/tests/unit_tests/inference_client/test_inference_client.py b/tests/unit_tests/inference_client/test_inference_client.py index da8adc9..de1cf2e 100644 --- a/tests/unit_tests/inference_client/test_inference_client.py +++ b/tests/unit_tests/inference_client/test_inference_client.py @@ -116,6 +116,7 @@ def test_the_deployment_name_is_sliced_out_correctly(self, inference_client): STATUS_URL = 'https://inference.example.com/v1/status/my-deployment' RESULT_URL = 'https://inference.example.com/v1/result/my-deployment' REDIRECT_URL = 'https://inference.example.com/v1/moved' +MOVED_URL = f'{BASE_URL}/moved' EXECUTION_ID = 'exec-1' REDIRECT_STATUSES = [301, 302, 303, 307, 308] @@ -193,6 +194,229 @@ def test_session_returns_a_redirect_as_is(self, inference_client): assert response.status_code == 307 assert len(responses.calls) == 1 + def test_request_redirect_inside_deployment_by_default_failed(self, inference_client): + # arrange + responses.add(responses.GET, PREDICT_URL, status=307, headers={'Location': MOVED_URL}) + responses.add(responses.GET, MOVED_URL, json={}) + + # act + with pytest.raises(InferenceClientError, match='redirects are not followed'): + inference_client.get('predict') + + # assert + assert inference_client.follow_redirects is False + assert len(responses.calls) == 1 + + @pytest.mark.parametrize( + ('path', 'location', 'hint'), + [ + ('predict', f'{PREDICT_URL}/', 'add a trailing slash to `path`'), + ('predict', '/predict/', 'add a trailing slash to `path`'), + ('predict/', PREDICT_URL, 'remove the trailing slash from `path`'), + ('predict/', '/predict', 'remove the trailing slash from `path`'), + ], + ) + def test_request_redirect_trailing_slash_failed(self, inference_client, path, location, hint): + # arrange + responses.add( + responses.GET, f'{BASE_URL}/{path}', status=307, headers={'Location': location} + ) + + # act + with pytest.raises(InferenceClientError) as excinfo: + inference_client.get(path) + + # assert + assert str(excinfo.value).endswith(hint) + + def test_request_redirect_to_another_path_has_no_hint_failed(self, inference_client): + # arrange + responses.add(responses.GET, PREDICT_URL, status=307, headers={'Location': MOVED_URL}) + + # act + with pytest.raises(InferenceClientError) as excinfo: + inference_client.get('predict') + + # assert + assert str(excinfo.value).endswith('redirects are not followed') + + +@pytest.fixture +def following_client(): + return InferenceClient( + inference_key='key-123', endpoint_base_url=BASE_URL, follow_redirects=True + ) + + +class TestFollowRedirects: + """With `follow_redirects=True`, only a redirect inside the deployment is followed.""" + + @pytest.mark.parametrize( + ('location', 'target'), + [ + (MOVED_URL, MOVED_URL), + ('/v1/my-deployment/moved', MOVED_URL), + ('moved', MOVED_URL), + ('/v1/my-deployment/a/b?q=1', f'{BASE_URL}/a/b?q=1'), + ('/v1/my-deployment', BASE_URL), + ], + ) + @pytest.mark.parametrize('status', REDIRECT_STATUSES) + def test_follow_redirect_inside_deployment_successful( + self, following_client, status, location, target + ): + # arrange + responses.add(responses.GET, PREDICT_URL, status=status, headers={'Location': location}) + responses.add( + responses.GET, + target, + json={'out': 1}, + match=[matchers.header_matcher({'Authorization': 'Bearer key-123'})], + ) + + # act + response = following_client.get('predict') + + # assert + assert response.json() == {'out': 1} + assert len(responses.calls) == 2 + assert responses.calls[1].request.url == target + + @pytest.mark.parametrize('status', [307, 308]) + def test_follow_redirect_keeps_method_and_body_successful(self, following_client, status): + # arrange + responses.add(responses.POST, PREDICT_URL, status=status, headers={'Location': MOVED_URL}) + responses.add( + responses.POST, + MOVED_URL, + json={'out': 1}, + match=[matchers.json_params_matcher({'a': 1})], + ) + + # act + response = following_client.run_sync({'a': 1}, path='predict') + + # assert + assert response.output() == {'out': 1} + assert responses.calls[1].request.method == 'POST' + + @pytest.mark.parametrize('status', [301, 302, 303]) + def test_follow_redirect_changes_post_to_get_successful(self, following_client, status): + # arrange + responses.add(responses.POST, PREDICT_URL, status=status, headers={'Location': MOVED_URL}) + responses.add(responses.GET, MOVED_URL, json={'out': 1}) + + # act + response = following_client.run_sync({'a': 1}, path='predict') + + # assert + assert response.output() == {'out': 1} + assert responses.calls[1].request.method == 'GET' + assert responses.calls[1].request.body is None + + @pytest.mark.parametrize( + 'location', + [ + # another deployment + 'https://inference.example.com/v1/other-deployment/moved', + '/v1/other-deployment/moved', + '/v1/moved', + # a deployment whose name starts with this one + '/v1/my-deployment-2/moved', + 'https://inference.example.com/v1/my-deployment-2', + # relative path segments, plain and encoded + '../other-deployment/moved', + '/v1/my-deployment/../other-deployment/moved', + '/v1/my-deployment/%2e%2e/other-deployment/moved', + '/v1/my-deployment/%2E%2E/other-deployment/moved', + '/v1/my-deployment/..%2Fother-deployment/moved', + '/v1/my-deployment/a/..%2F..%2F..%2Fother-deployment', + '/v1/my-deployment/./moved', + # another host + 'https://other.example.net/v1/my-deployment/moved', + '//other.example.net/v1/my-deployment/moved', + # another scheme + 'http://inference.example.com/v1/my-deployment/moved', + # another port + 'https://inference.example.com:8443/v1/my-deployment/moved', + 'https://inference.example.com:bad/v1/my-deployment/moved', + # user info + 'https://user@inference.example.com/v1/my-deployment/moved', + ], + ) + @pytest.mark.parametrize('status', REDIRECT_STATUSES) + def test_follow_redirect_outside_deployment_failed(self, following_client, status, location): + # arrange + responses.add(responses.POST, PREDICT_URL, status=status, headers={'Location': location}) + + # act + with pytest.raises(InferenceClientError, match=rf'redirect \({status}\)') as excinfo: + following_client.run_sync({'a': 1}, path='predict') + + # assert + assert 'outside the deployment' in str(excinfo.value) + assert len(responses.calls) == 1 + + def test_follow_redirect_stops_at_rejected_hop_failed(self, following_client): + # arrange + location = 'https://other.example.net/moved' + responses.add(responses.GET, PREDICT_URL, status=307, headers={'Location': MOVED_URL}) + responses.add(responses.GET, MOVED_URL, status=307, headers={'Location': location}) + + # act + with pytest.raises(InferenceClientError, match=location): + following_client.get('predict') + + # assert + assert len(responses.calls) == 2 + + def test_follow_redirect_hop_limit_failed(self, following_client): + # arrange + responses.add( + responses.GET, PREDICT_URL, status=307, headers={'Location': f'{BASE_URL}/hop-1'} + ) + for hop in range(1, 10): + responses.add( + responses.GET, + f'{BASE_URL}/hop-{hop}', + status=307, + headers={'Location': f'{BASE_URL}/hop-{hop + 1}'}, + ) + + # act + with pytest.raises(InferenceClientError, match='Exceeded 5 redirects'): + following_client.get('predict') + + # assert + assert len(responses.calls) == 6 + + @pytest.mark.parametrize('entry_point', ['status_json', 'result']) + @pytest.mark.parametrize('status', REDIRECT_STATUSES) + def test_async_request_redirect_failed(self, following_client, entry_point, status): + # arrange + _, url, call = ENTRY_POINTS[entry_point] + responses.add(responses.GET, url, status=status, headers={'Location': MOVED_URL}) + responses.add(responses.GET, MOVED_URL, json={'Status': 'Completed'}) + + # act + with pytest.raises(InferenceClientError, match='redirects are not followed'): + call(following_client) + + # assert + assert len(responses.calls) == 1 + + def test_session_returns_a_redirect_outside_deployment_as_is(self, following_client): + # arrange + responses.add(responses.GET, PREDICT_URL, status=307, headers={'Location': REDIRECT_URL}) + responses.add(responses.GET, REDIRECT_URL, json={}) + + # act + response = following_client._session.get(PREDICT_URL, allow_redirects=True) + + # assert + assert response.status_code == 307 + assert len(responses.calls) == 1 + class TestRequests: """Successful inference requests.""" diff --git a/verda/inference_client/_inference_client.py b/verda/inference_client/_inference_client.py index 876e24d..34a24b7 100644 --- a/verda/inference_client/_inference_client.py +++ b/verda/inference_client/_inference_client.py @@ -16,7 +16,7 @@ from dataclasses import dataclass from enum import Enum from typing import Any -from urllib.parse import urlparse +from urllib.parse import urljoin, urlparse, urlsplit import requests from dataclasses_json import Undefined, dataclass_json # type: ignore @@ -119,23 +119,123 @@ def stream(self, chunk_size: int = 512, as_text: bool = True) -> Generator[Any, yield chunk -class _NoRedirectSession(requests.Session): - """A session that never follows a redirect, even if asked to. +# The most redirects a deployment request follows when `follow_redirects` is on. +_MAX_REDIRECTS = 5 + +_DEFAULT_PORTS = {'http': 80, 'https': 443} - Requests are sent with ``allow_redirects=False``. This is the backstop for a call - that forgets: the 3xx response is returned as-is instead of being followed. - """ - def get_redirect_target(self, resp: requests.Response) -> None: # noqa: ARG002 - """Report no redirect target, so ``resolve_redirects`` has nothing to follow.""" +def _origin(url: str) -> tuple[str, str | None, int | None, str | None, str | None] | None: + """Return the scheme, host, port and user info of a url, or None if the port is invalid.""" + parts = urlsplit(url) + scheme = parts.scheme.lower() + try: + port = parts.port or _DEFAULT_PORTS.get(scheme) + except ValueError: return None + return scheme, parts.hostname, port, parts.username, parts.password + + +def _is_inside_deployment(endpoint_base_url: str, response_url: str, location: str) -> bool: + """Whether a redirect ``Location`` stays inside the deployment. + + The target must have the same scheme, host and port as ``endpoint_base_url``, and + its path must be the deployment path or start with ``/``. The raw + ``Location`` path must have no relative segment: ``requests`` decodes ``%2E`` before + it resolves the target, so a prefix check on the resolved url alone misses + ``//%2E%2E/``. + + Args: + endpoint_base_url: The deployment's base url, without a trailing slash. + response_url: The url of the request that returned the redirect. + location: The ``Location`` header value. + + Returns: + True if the redirect may be followed. + """ + if has_relative_path_segment(urlsplit(location).path): + return False + target = urljoin(response_url, location) + base_origin = _origin(endpoint_base_url) + if base_origin is None or _origin(target) != base_origin: + return False + deployment_path = urlsplit(endpoint_base_url).path + target_path = urlsplit(target).path + return target_path == deployment_path or target_path.startswith(deployment_path + '/') + + +def _trailing_slash_hint(response_url: str, location: str | None) -> str: + """Suggest a ``path`` fix when a redirect only adds or removes a trailing slash. + + The ``Location`` may lack the deployment prefix, so the target path only has to be + a suffix of the request path, ignoring the trailing slash. + + Args: + response_url: The url of the request that returned the redirect. + location: The ``Location`` header value, if any. + + Returns: + The hint to append to the error message, or an empty string. + """ + if not location: + return '' + request_path = urlsplit(response_url).path + target_path = urlsplit(urljoin(response_url, location)).path + stripped = target_path.rstrip('/') + if not stripped or request_path.endswith('/') == target_path.endswith('/'): + return '' + if not request_path.rstrip('/').endswith(stripped): + return '' + if target_path.endswith('/'): + return '; add a trailing slash to `path`' + return '; remove the trailing slash from `path`' + + +class _RedirectPolicySession(requests.Session): + """A session that follows a redirect only inside the deployment, and only if enabled. + + Requests are sent with ``allow_redirects=False`` unless the client enables + ``follow_redirects``. This is the backstop for a call that forgets: without an + ``endpoint_base_url`` no redirect target is reported, so the 3xx response is + returned as-is instead of being followed. + """ + + def __init__(self, endpoint_base_url: str | None = None) -> None: + """Initialize the session. + + Args: + endpoint_base_url: The deployment's base url. If set, a redirect inside + the deployment is followed, up to ``_MAX_REDIRECTS`` times. + """ + super().__init__() + self.endpoint_base_url = endpoint_base_url + self.max_redirects = _MAX_REDIRECTS + + def get_redirect_target(self, resp: requests.Response) -> str | None: + """Report the redirect target only if it stays inside the deployment. + + ``resolve_redirects`` stops when this returns None, and the 3xx response is + returned to the caller. + """ + if self.endpoint_base_url is None: + return None + location = super().get_redirect_target(resp) + if location is None or not _is_inside_deployment( + self.endpoint_base_url, resp.url, location + ): + return None + return location class InferenceClient: """Inference client.""" def __init__( - self, inference_key: str, endpoint_base_url: str, timeout_seconds: int = 60 * 5 + self, + inference_key: str, + endpoint_base_url: str, + timeout_seconds: int = 60 * 5, + follow_redirects: bool = False, ) -> None: """Initialize the InferenceClient. @@ -143,6 +243,10 @@ def __init__( inference_key: The authentication key for the API endpoint_base_url: The base URL for the API timeout_seconds: Request timeout in seconds + follow_redirects: Follow a redirect whose target is inside this deployment: + same scheme, host and port, and a path under ``endpoint_base_url``. + Any other redirect raises ``InferenceClientError``. Async status and + result requests never follow a redirect. Defaults to False. Raises: InferenceClientError: If the parameters are invalid @@ -174,7 +278,8 @@ def __init__( self.base_domain = self.endpoint_base_url[: self.endpoint_base_url.rindex('/')] self.deployment_name = self.endpoint_base_url[self.endpoint_base_url.rindex('/') + 1 :] self.timeout_seconds = timeout_seconds - self._session = _NoRedirectSession() + self.follow_redirects = follow_redirects + self._session = _RedirectPolicySession(self.endpoint_base_url if follow_redirects else None) self._global_headers = { 'Authorization': f'Bearer {inference_key}', 'Content-Type': 'application/json', @@ -282,25 +387,35 @@ def _make_request(self, method: str, path: str, **kwargs) -> requests.Response: self._build_url(path), path, timeout, + follow_redirects=self.follow_redirects, headers=self._build_request_headers(kwargs.pop('headers', None)), **kwargs, ) def _send( - self, method: str, url: str, path: str, timeout: float | None, **kwargs + self, + method: str, + url: str, + path: str, + timeout: float | None, + follow_redirects: bool = False, + **kwargs, ) -> requests.Response: - """Send a request without following redirects. + """Send a request, following a redirect only inside the deployment. - Every request carrying the inference key goes through here. A redirect is - not followed: ``requests`` keeps the ``Authorization`` header, method and body - on a same-host redirect, so a ``Location`` from the response would retarget - the request at a path that never went through ``_build_url``. + Every request carrying the inference key goes through here. By default a + redirect is not followed: ``requests`` keeps the ``Authorization`` header, + method and body on a same-host redirect, so a ``Location`` from the response + would retarget the request at a path that never went through ``_build_url``. + With ``follow_redirects``, the session follows only a target inside the + deployment, and any other 3xx is returned here and raised. Args: method: HTTP method to use url: Full request URL path: The path as the caller gave it, for error messages timeout: Request timeout in seconds + follow_redirects: Follow a redirect whose target is inside the deployment **kwargs: Additional arguments to pass to the request Returns: @@ -308,22 +423,29 @@ def _send( Raises: InferenceClientError: If the request fails or the response is a redirect + that is not followed """ try: response = self._session.request( method=method, url=url, timeout=timeout, - allow_redirects=False, + allow_redirects=follow_redirects, **kwargs, ) # raise_for_status() lets a 3xx through. if 300 <= response.status_code < 400: location = response.headers.get('Location') response.close() + reason = ( + 'the target is outside the deployment and was not followed' + if follow_redirects and location + else 'redirects are not followed' + ) raise InferenceClientError( f'Request to {path} returned a redirect ({response.status_code}) ' - f'to {location!r}; redirects are not followed' + f'to {location!r}; {reason}' + f'{_trailing_slash_hint(response.url, location)}' ) response.raise_for_status() return response @@ -584,7 +706,11 @@ def status(self) -> AsyncStatus: return self._status def _get(self, url: str) -> requests.Response: - """GET an async status or result url for this execution.""" + """GET an async status or result url for this execution. + + These urls belong to the gateway, not the deployment, so a redirect is never + followed, even if the client enables ``follow_redirects``. + """ client = self._inference_client return client._send( 'GET', From 67ea5488e24efa63ba0971b3e76bcaf4f7b3679e Mon Sep 17 00:00:00 2001 From: Dmitrii Iutkin Date: Thu, 29 Oct 2026 17:16:33 +0200 Subject: [PATCH 03/10] Check the redirect target path with the inference path rules With follow_redirects on, the part of the target path under the deployment must now pass check_request_path with the same allowlist as path. Forms such as `..;/`, `%`-escapes and `\` are rejected. The query string is not checked. Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 2 +- .../inference_client/test_inference_client.py | 7 +++++++ verda/inference_client/_inference_client.py | 12 ++++++++++-- 3 files changed, 18 insertions(+), 3 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 311c53c..53149dd 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -107,7 +107,7 @@ All five verbs delegate to a single private `_request`, which is where the url i **Never follow a redirect.** Every request that carries credentials is sent with `allow_redirects=False`, and a 3xx raises: `handle_error` for `HTTPClient` and the auth token calls, and `InferenceClient._send` for inference. `requests` keeps `Authorization` on a same-host redirect, and a 307/308 also keeps the method and body. The `Location` target never went through path validation. Any new send site must pass `allow_redirects=False` and go through the existing error check. Inference requests, including `AsyncInferenceExecution` status and result, go through `InferenceClient._send`, never `_session` directly. -The one exception is `InferenceClient(follow_redirects=True)`, an opt-in for deployment calls only. `_send` then passes `allow_redirects=True`, and `_RedirectPolicySession.get_redirect_target` reports a target only if `_is_inside_deployment` accepts it: same scheme, host and port as `endpoint_base_url`, a path equal to the deployment path or under `/`, and a raw `Location` path that passes `has_relative_path_segment`. A rejected target stops `resolve_redirects`, and `_send` raises on the remaining 3xx. The session caps hops at 5. `AsyncInferenceExecution` status and result never follow, because those urls belong to the gateway. Do not add the flag to `HTTPClient` or the auth token calls. +The one exception is `InferenceClient(follow_redirects=True)`, an opt-in for deployment calls only. `_send` then passes `allow_redirects=True`, and `_RedirectPolicySession.get_redirect_target` reports a target only if `_is_inside_deployment` accepts it: same scheme, host and port as `endpoint_base_url`, a path equal to the deployment path or under `/`, a raw `Location` path that passes `has_relative_path_segment`, and a target path under the deployment path that passes `check_request_path` with `_ALLOWED_IN_INFERENCE_PATH`, the same check as `path`. A rejected target stops `resolve_redirects`, and `_send` raises on the remaining 3xx. The session caps hops at 5. `AsyncInferenceExecution` status and result never follow, because those urls belong to the gateway. Do not add the flag to `HTTPClient` or the auth token calls. The check is `_encode_path_segment` in `verda/http_client/_http_client.py`; `path_params` is the only supported way to put a caller-supplied value into a path. `_add_base_url` re-asserts the same allowlist on the finished path, as a backstop for a call site that skips `path_params`. diff --git a/tests/unit_tests/inference_client/test_inference_client.py b/tests/unit_tests/inference_client/test_inference_client.py index 6444c78..0950d31 100644 --- a/tests/unit_tests/inference_client/test_inference_client.py +++ b/tests/unit_tests/inference_client/test_inference_client.py @@ -390,6 +390,7 @@ class TestFollowRedirects: ('moved', MOVED_URL), ('/v1/my-deployment/a/b?q=1', f'{BASE_URL}/a/b?q=1'), ('/v1/my-deployment', BASE_URL), + ('/v1/my-deployment/v1/models/m:predict', f'{BASE_URL}/v1/models/m:predict'), ], ) @pytest.mark.parametrize('status', REDIRECT_STATUSES) @@ -463,6 +464,12 @@ def test_follow_redirect_changes_post_to_get_successful(self, following_client, '/v1/my-deployment/..%2Fother-deployment/moved', '/v1/my-deployment/a/..%2F..%2F..%2Fother-deployment', '/v1/my-deployment/./moved', + '/v1/my-deployment/..;/other-deployment/moved', + '/v1/my-deployment/..%252f..%252fother-deployment', + # characters that `path` does not allow + '/v1/my-deployment/a%20b', + '/v1/my-deployment/a;b', + '/v1/my-deployment/a\\b', # another host 'https://other.example.net/v1/my-deployment/moved', '//other.example.net/v1/my-deployment/moved', diff --git a/verda/inference_client/_inference_client.py b/verda/inference_client/_inference_client.py index 2bad4f9..e33d484 100644 --- a/verda/inference_client/_inference_client.py +++ b/verda/inference_client/_inference_client.py @@ -148,7 +148,9 @@ def _is_inside_deployment(endpoint_base_url: str, response_url: str, location: s its path must be the deployment path or start with ``/``. The raw ``Location`` path must have no relative segment: ``requests`` decodes ``%2E`` before it resolves the target, so a prefix check on the resolved url alone misses - ``//%2E%2E/``. + ``//%2E%2E/``. The rest of the target path must pass the same + check as ``path`` in ``_build_url``, which also refuses forms such as ``..;/`` and + ``..%252f``. The query string is not checked: it does not change the target path. Args: endpoint_base_url: The deployment's base url, without a trailing slash. @@ -166,7 +168,13 @@ def _is_inside_deployment(endpoint_base_url: str, response_url: str, location: s return False deployment_path = urlsplit(endpoint_base_url).path target_path = urlsplit(target).path - return target_path == deployment_path or target_path.startswith(deployment_path + '/') + if target_path != deployment_path and not target_path.startswith(deployment_path + '/'): + return False + try: + check_request_path(target_path[len(deployment_path) :], _ALLOWED_IN_INFERENCE_PATH) + except ValueError: + return False + return True def _trailing_slash_hint(response_url: str, location: str | None) -> str: From 784dcc16fa4f4d5afee6f105ca7e748303c380d1 Mon Sep 17 00:00:00 2001 From: Dmitrii Iutkin Date: Fri, 30 Oct 2026 10:09:10 +0200 Subject: [PATCH 04/10] Add an example and docs page for follow_redirects The example looks up a deployment and creates an InferenceClient with its endpoint_base_url and follow_redirects=True. Deployment.run_sync and run always use the default, so this is the way to turn on the option for a deployment. Co-Authored-By: Claude Opus 5.5 --- docs/source/examples/containers/index.rst | 3 +- .../containers/inference_redirects.rst | 19 ++++++ .../calling_the_endpoint_with_redirects.py | 60 +++++++++++++++++++ 3 files changed, 81 insertions(+), 1 deletion(-) create mode 100644 docs/source/examples/containers/inference_redirects.rst create mode 100644 examples/containers/calling_the_endpoint_with_redirects.py diff --git a/docs/source/examples/containers/index.rst b/docs/source/examples/containers/index.rst index 324ce2f..8bd7359 100644 --- a/docs/source/examples/containers/index.rst +++ b/docs/source/examples/containers/index.rst @@ -17,4 +17,5 @@ This section contains examples demonstrating how to work with containers in Verd scaling inference_async inference_sync - inference_minimal \ No newline at end of file + inference_minimal + inference_redirects diff --git a/docs/source/examples/containers/inference_redirects.rst b/docs/source/examples/containers/inference_redirects.rst new file mode 100644 index 0000000..3507741 --- /dev/null +++ b/docs/source/examples/containers/inference_redirects.rst @@ -0,0 +1,19 @@ +Following redirects inside a deployment +======================================= + +By default, ``InferenceClient`` does not follow HTTP redirects. A 3xx response raises ``InferenceClientError``, and the error message shows the ``Location`` value. + +Set ``follow_redirects=True`` if your container sends redirects inside its own deployment. The client then follows a redirect only if all of these are true: + +- The target has the same scheme, host and port as ``endpoint_base_url``. +- The target path is the deployment path, or it starts with the deployment path and ``/``. +- The target path passes the same check as ``path``. +- The client followed fewer than 5 redirects. + +The client raises ``InferenceClientError`` for any other redirect. ``AsyncInferenceExecution.status_json()`` and ``result()`` never follow a redirect. + +``Deployment.run_sync`` and ``Deployment.run`` always use the default. To follow redirects, create an ``InferenceClient`` with the deployment's ``endpoint_base_url``, as this example shows. + +.. literalinclude:: ../../../../examples/containers/calling_the_endpoint_with_redirects.py + :language: python + :caption: Following redirects inside a deployment diff --git a/examples/containers/calling_the_endpoint_with_redirects.py b/examples/containers/calling_the_endpoint_with_redirects.py new file mode 100644 index 0000000..a889301 --- /dev/null +++ b/examples/containers/calling_the_endpoint_with_redirects.py @@ -0,0 +1,60 @@ +# Copyright 2026 Verda Cloud Oy +# +# Licensed 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 os + +from verda import VerdaClient +from verda.inference_client import InferenceClient, InferenceClientError + +# Configuration - replace with your deployment name +DEPLOYMENT_NAME = os.environ.get('VERDA_DEPLOYMENT_NAME') + +# Client id and secret authenticate calls to the Verda API, such as the deployment lookup +CLIENT_ID = os.environ.get('VERDA_CLIENT_ID') +CLIENT_SECRET = os.environ.get('VERDA_CLIENT_SECRET') + +# The inference key authenticates calls to the deployment endpoint +INFERENCE_KEY = os.environ.get('VERDA_INFERENCE_KEY') + +# Verda client instance, used only to look up the deployment +verda = VerdaClient(CLIENT_ID, CLIENT_SECRET) + +# Get the deployment, to read its endpoint base url +deployment = verda.containers.get_deployment_by_name(DEPLOYMENT_NAME) + +# By default the inference client does not follow redirects, and a 3xx response +# raises InferenceClientError. With follow_redirects=True, the client follows a +# redirect only if the target is inside this deployment: the same scheme, host and +# port, and a path under endpoint_base_url. It follows at most 5 redirects. +with InferenceClient( + inference_key=INFERENCE_KEY, + endpoint_base_url=deployment.endpoint_base_url, + follow_redirects=True, +) as inference_client: + # Make a synchronous request to the endpoint. + # This example demonstrates calling a SGLang deployment which serves LLMs using an OpenAI-compatible API format + data = { + 'model': 'deepseek-ai/deepseek-llm-7b-chat', + 'prompt': 'Write a haiku about the sea.', + 'max_tokens': 64, + 'temperature': 0.7, + } + + try: + response = inference_client.run_sync(data=data, path='v1/completions') + print(response.output()) + except InferenceClientError as e: + # A redirect to a target outside the deployment is not followed. The error + # message shows the Location value, and a hint if only a trailing slash differs. + print(f'Request failed: {e}') From 576cd40e6c481e1dd553d42cf0a1d53d12ec721f Mon Sep 17 00:00:00 2001 From: Dmitrii Iutkin Date: Fri, 30 Oct 2026 10:15:16 +0200 Subject: [PATCH 05/10] Fix Sphinx docs build errors Point the service API pages at the public import paths, such as verda.containers.ContainersService. The old paths named modules that no longer exist, so autodoc could not import them. Include CHANGELOG.md and CONTRIBUTING.md with the recommonmark parser. The pages referenced .rst files that no longer exist, so they had no content and no title. Add inference_minimal_async to the container examples toctree. Co-Authored-By: Claude Opus 5.5 --- docs/source/api/services/balance.rst | 4 ++-- docs/source/api/services/containers.rst | 4 ++-- docs/source/api/services/images.rst | 4 ++-- docs/source/api/services/instance_types.rst | 4 ++-- docs/source/api/services/instances.rst | 4 ++-- docs/source/api/services/ssh_keys.rst | 4 ++-- docs/source/api/services/startup_scripts.rst | 4 ++-- docs/source/api/services/volume_types.rst | 4 ++-- docs/source/api/services/volumes.rst | 4 ++-- docs/source/changelog.rst | 3 ++- docs/source/contributing.rst | 3 ++- docs/source/examples/containers/index.rst | 1 + 12 files changed, 23 insertions(+), 20 deletions(-) diff --git a/docs/source/api/services/balance.rst b/docs/source/api/services/balance.rst index e310ae1..f777d22 100644 --- a/docs/source/api/services/balance.rst +++ b/docs/source/api/services/balance.rst @@ -1,8 +1,8 @@ Balance ======= -.. autoclass:: verda.balance.balance.BalanceService +.. autoclass:: verda.balance.BalanceService :members: -.. autoclass:: verda.balance.balance.Balance +.. autoclass:: verda.balance.Balance :members: \ No newline at end of file diff --git a/docs/source/api/services/containers.rst b/docs/source/api/services/containers.rst index c92fa2d..19ea502 100644 --- a/docs/source/api/services/containers.rst +++ b/docs/source/api/services/containers.rst @@ -1,9 +1,9 @@ Containers ========== -.. autoclass:: verda.containers.containers.ContainersService +.. autoclass:: verda.containers.ContainersService :members: -.. autoclass:: verda.containers.containers.Container +.. autoclass:: verda.containers.Container :members: diff --git a/docs/source/api/services/images.rst b/docs/source/api/services/images.rst index 621f17e..378b5ad 100644 --- a/docs/source/api/services/images.rst +++ b/docs/source/api/services/images.rst @@ -1,8 +1,8 @@ Images ====== -.. autoclass:: verda.images.images.ImagesService +.. autoclass:: verda.images.ImagesService :members: -.. autoclass:: verda.images.images.Image +.. autoclass:: verda.images.Image :members: \ No newline at end of file diff --git a/docs/source/api/services/instance_types.rst b/docs/source/api/services/instance_types.rst index 1ee1541..599fc1d 100644 --- a/docs/source/api/services/instance_types.rst +++ b/docs/source/api/services/instance_types.rst @@ -1,8 +1,8 @@ Instance Types ============== -.. autoclass:: verda.instance_types.instance_types.InstanceTypesService +.. autoclass:: verda.instance_types.InstanceTypesService :members: -.. autoclass:: verda.instance_types.instance_types.InstanceType +.. autoclass:: verda.instance_types.InstanceType :members: \ No newline at end of file diff --git a/docs/source/api/services/instances.rst b/docs/source/api/services/instances.rst index b4c1409..08656a0 100644 --- a/docs/source/api/services/instances.rst +++ b/docs/source/api/services/instances.rst @@ -1,8 +1,8 @@ Instances ========= -.. autoclass:: verda.instances.instances.InstancesService +.. autoclass:: verda.instances.InstancesService :members: -.. autoclass:: verda.instances.instances.Instance +.. autoclass:: verda.instances.Instance :members: \ No newline at end of file diff --git a/docs/source/api/services/ssh_keys.rst b/docs/source/api/services/ssh_keys.rst index ae9da7f..16432a6 100644 --- a/docs/source/api/services/ssh_keys.rst +++ b/docs/source/api/services/ssh_keys.rst @@ -1,8 +1,8 @@ SSH Keys ======== -.. autoclass:: verda.ssh_keys.ssh_keys.SSHKeysService +.. autoclass:: verda.ssh_keys.SSHKeysService :members: -.. autoclass:: verda.ssh_keys.ssh_keys.SSHKey +.. autoclass:: verda.ssh_keys.SSHKey :members: \ No newline at end of file diff --git a/docs/source/api/services/startup_scripts.rst b/docs/source/api/services/startup_scripts.rst index d9b89ed..171f18d 100644 --- a/docs/source/api/services/startup_scripts.rst +++ b/docs/source/api/services/startup_scripts.rst @@ -1,8 +1,8 @@ Startup Scripts =============== -.. autoclass:: verda.startup_scripts.startup_scripts.StartupScriptsService +.. autoclass:: verda.startup_scripts.StartupScriptsService :members: -.. autoclass:: verda.startup_scripts.startup_scripts.StartupScript +.. autoclass:: verda.startup_scripts.StartupScript :members: \ No newline at end of file diff --git a/docs/source/api/services/volume_types.rst b/docs/source/api/services/volume_types.rst index cdf70cc..9a1b998 100644 --- a/docs/source/api/services/volume_types.rst +++ b/docs/source/api/services/volume_types.rst @@ -1,8 +1,8 @@ Volume Types ============ -.. autoclass:: verda.volume_types.volume_types.VolumeTypesService +.. autoclass:: verda.volume_types.VolumeTypesService :members: -.. autoclass:: verda.volume_types.volume_types.VolumeType +.. autoclass:: verda.volume_types.VolumeType :members: \ No newline at end of file diff --git a/docs/source/api/services/volumes.rst b/docs/source/api/services/volumes.rst index b6c850a..ac06bd6 100644 --- a/docs/source/api/services/volumes.rst +++ b/docs/source/api/services/volumes.rst @@ -1,8 +1,8 @@ Volumes ======= -.. autoclass:: verda.volumes.volumes.VolumesService +.. autoclass:: verda.volumes.VolumesService :members: -.. autoclass:: verda.volumes.volumes.Volume +.. autoclass:: verda.volumes.Volume :members: \ No newline at end of file diff --git a/docs/source/changelog.rst b/docs/source/changelog.rst index 6f90edd..b89da39 100644 --- a/docs/source/changelog.rst +++ b/docs/source/changelog.rst @@ -1 +1,2 @@ -.. include:: ../../CHANGELOG.rst \ No newline at end of file +.. include:: ../../CHANGELOG.md + :parser: recommonmark diff --git a/docs/source/contributing.rst b/docs/source/contributing.rst index 8cb3146..370e0c0 100644 --- a/docs/source/contributing.rst +++ b/docs/source/contributing.rst @@ -1 +1,2 @@ -.. include:: ../../CONTRIBUTING.rst \ No newline at end of file +.. include:: ../../CONTRIBUTING.md + :parser: recommonmark diff --git a/docs/source/examples/containers/index.rst b/docs/source/examples/containers/index.rst index 8bd7359..a4d55d6 100644 --- a/docs/source/examples/containers/index.rst +++ b/docs/source/examples/containers/index.rst @@ -18,4 +18,5 @@ This section contains examples demonstrating how to work with containers in Verd inference_async inference_sync inference_minimal + inference_minimal_async inference_redirects From 847495104f9e2242234b18d42bdcd9379d15c8fa Mon Sep 17 00:00:00 2001 From: Dmitrii Iutkin Date: Fri, 30 Oct 2026 10:19:10 +0200 Subject: [PATCH 06/10] Revert "Fix Sphinx docs build errors" This reverts commit 576cd40e6c481e1dd553d42cf0a1d53d12ec721f. The docs build fix goes in a separate pull request. Co-Authored-By: Claude Opus 5.5 --- docs/source/api/services/balance.rst | 4 ++-- docs/source/api/services/containers.rst | 4 ++-- docs/source/api/services/images.rst | 4 ++-- docs/source/api/services/instance_types.rst | 4 ++-- docs/source/api/services/instances.rst | 4 ++-- docs/source/api/services/ssh_keys.rst | 4 ++-- docs/source/api/services/startup_scripts.rst | 4 ++-- docs/source/api/services/volume_types.rst | 4 ++-- docs/source/api/services/volumes.rst | 4 ++-- docs/source/changelog.rst | 3 +-- docs/source/contributing.rst | 3 +-- docs/source/examples/containers/index.rst | 1 - 12 files changed, 20 insertions(+), 23 deletions(-) diff --git a/docs/source/api/services/balance.rst b/docs/source/api/services/balance.rst index f777d22..e310ae1 100644 --- a/docs/source/api/services/balance.rst +++ b/docs/source/api/services/balance.rst @@ -1,8 +1,8 @@ Balance ======= -.. autoclass:: verda.balance.BalanceService +.. autoclass:: verda.balance.balance.BalanceService :members: -.. autoclass:: verda.balance.Balance +.. autoclass:: verda.balance.balance.Balance :members: \ No newline at end of file diff --git a/docs/source/api/services/containers.rst b/docs/source/api/services/containers.rst index 19ea502..c92fa2d 100644 --- a/docs/source/api/services/containers.rst +++ b/docs/source/api/services/containers.rst @@ -1,9 +1,9 @@ Containers ========== -.. autoclass:: verda.containers.ContainersService +.. autoclass:: verda.containers.containers.ContainersService :members: -.. autoclass:: verda.containers.Container +.. autoclass:: verda.containers.containers.Container :members: diff --git a/docs/source/api/services/images.rst b/docs/source/api/services/images.rst index 378b5ad..621f17e 100644 --- a/docs/source/api/services/images.rst +++ b/docs/source/api/services/images.rst @@ -1,8 +1,8 @@ Images ====== -.. autoclass:: verda.images.ImagesService +.. autoclass:: verda.images.images.ImagesService :members: -.. autoclass:: verda.images.Image +.. autoclass:: verda.images.images.Image :members: \ No newline at end of file diff --git a/docs/source/api/services/instance_types.rst b/docs/source/api/services/instance_types.rst index 599fc1d..1ee1541 100644 --- a/docs/source/api/services/instance_types.rst +++ b/docs/source/api/services/instance_types.rst @@ -1,8 +1,8 @@ Instance Types ============== -.. autoclass:: verda.instance_types.InstanceTypesService +.. autoclass:: verda.instance_types.instance_types.InstanceTypesService :members: -.. autoclass:: verda.instance_types.InstanceType +.. autoclass:: verda.instance_types.instance_types.InstanceType :members: \ No newline at end of file diff --git a/docs/source/api/services/instances.rst b/docs/source/api/services/instances.rst index 08656a0..b4c1409 100644 --- a/docs/source/api/services/instances.rst +++ b/docs/source/api/services/instances.rst @@ -1,8 +1,8 @@ Instances ========= -.. autoclass:: verda.instances.InstancesService +.. autoclass:: verda.instances.instances.InstancesService :members: -.. autoclass:: verda.instances.Instance +.. autoclass:: verda.instances.instances.Instance :members: \ No newline at end of file diff --git a/docs/source/api/services/ssh_keys.rst b/docs/source/api/services/ssh_keys.rst index 16432a6..ae9da7f 100644 --- a/docs/source/api/services/ssh_keys.rst +++ b/docs/source/api/services/ssh_keys.rst @@ -1,8 +1,8 @@ SSH Keys ======== -.. autoclass:: verda.ssh_keys.SSHKeysService +.. autoclass:: verda.ssh_keys.ssh_keys.SSHKeysService :members: -.. autoclass:: verda.ssh_keys.SSHKey +.. autoclass:: verda.ssh_keys.ssh_keys.SSHKey :members: \ No newline at end of file diff --git a/docs/source/api/services/startup_scripts.rst b/docs/source/api/services/startup_scripts.rst index 171f18d..d9b89ed 100644 --- a/docs/source/api/services/startup_scripts.rst +++ b/docs/source/api/services/startup_scripts.rst @@ -1,8 +1,8 @@ Startup Scripts =============== -.. autoclass:: verda.startup_scripts.StartupScriptsService +.. autoclass:: verda.startup_scripts.startup_scripts.StartupScriptsService :members: -.. autoclass:: verda.startup_scripts.StartupScript +.. autoclass:: verda.startup_scripts.startup_scripts.StartupScript :members: \ No newline at end of file diff --git a/docs/source/api/services/volume_types.rst b/docs/source/api/services/volume_types.rst index 9a1b998..cdf70cc 100644 --- a/docs/source/api/services/volume_types.rst +++ b/docs/source/api/services/volume_types.rst @@ -1,8 +1,8 @@ Volume Types ============ -.. autoclass:: verda.volume_types.VolumeTypesService +.. autoclass:: verda.volume_types.volume_types.VolumeTypesService :members: -.. autoclass:: verda.volume_types.VolumeType +.. autoclass:: verda.volume_types.volume_types.VolumeType :members: \ No newline at end of file diff --git a/docs/source/api/services/volumes.rst b/docs/source/api/services/volumes.rst index ac06bd6..b6c850a 100644 --- a/docs/source/api/services/volumes.rst +++ b/docs/source/api/services/volumes.rst @@ -1,8 +1,8 @@ Volumes ======= -.. autoclass:: verda.volumes.VolumesService +.. autoclass:: verda.volumes.volumes.VolumesService :members: -.. autoclass:: verda.volumes.Volume +.. autoclass:: verda.volumes.volumes.Volume :members: \ No newline at end of file diff --git a/docs/source/changelog.rst b/docs/source/changelog.rst index b89da39..6f90edd 100644 --- a/docs/source/changelog.rst +++ b/docs/source/changelog.rst @@ -1,2 +1 @@ -.. include:: ../../CHANGELOG.md - :parser: recommonmark +.. include:: ../../CHANGELOG.rst \ No newline at end of file diff --git a/docs/source/contributing.rst b/docs/source/contributing.rst index 370e0c0..8cb3146 100644 --- a/docs/source/contributing.rst +++ b/docs/source/contributing.rst @@ -1,2 +1 @@ -.. include:: ../../CONTRIBUTING.md - :parser: recommonmark +.. include:: ../../CONTRIBUTING.rst \ No newline at end of file diff --git a/docs/source/examples/containers/index.rst b/docs/source/examples/containers/index.rst index a4d55d6..8bd7359 100644 --- a/docs/source/examples/containers/index.rst +++ b/docs/source/examples/containers/index.rst @@ -18,5 +18,4 @@ This section contains examples demonstrating how to work with containers in Verd inference_async inference_sync inference_minimal - inference_minimal_async inference_redirects From a7228ca246472cdff7fde7fe6cec40a4d328d282 Mon Sep 17 00:00:00 2001 From: Dmitrii Iutkin Date: Fri, 30 Oct 2026 11:07:37 +0200 Subject: [PATCH 07/10] Raise InferenceClientError for a malformed redirect Location A Location that urllib cannot parse, such as an unclosed IPv6 bracket, raised ValueError from the trailing-slash hint or the redirect check. The redirect check now rejects such a Location, and the hint is empty. The client then raises InferenceClientError, as for any other redirect. Tests: assert APIException.code in the redirect tests, rename tests to end in _successful or _failed, and move module constants to the top. Co-Authored-By: Claude Opus 5.5 --- .../authentication/test_authentication.py | 2 + .../http_client/test_http_client.py | 4 +- .../inference_client/test_inference_client.py | 113 +++++++++++------- verda/inference_client/_inference_client.py | 20 +++- 4 files changed, 87 insertions(+), 52 deletions(-) diff --git a/tests/unit_tests/authentication/test_authentication.py b/tests/unit_tests/authentication/test_authentication.py index b9ebb0b..845501f 100644 --- a/tests/unit_tests/authentication/test_authentication.py +++ b/tests/unit_tests/authentication/test_authentication.py @@ -256,6 +256,7 @@ def test_authenticate_redirect_failed(self, authentication_service, endpoint, st authentication_service.authenticate() # assert + assert excinfo.value.code is None assert f'unexpected redirect ({status})' in excinfo.value.message assert len(responses.calls) == 1 @@ -273,5 +274,6 @@ def test_refresh_redirect_failed(self, authentication_service, endpoint, status) authentication_service.refresh() # assert + assert excinfo.value.code is None assert f'unexpected redirect ({status})' in excinfo.value.message assert len(responses.calls) == 1 diff --git a/tests/unit_tests/http_client/test_http_client.py b/tests/unit_tests/http_client/test_http_client.py index 2ecd487..8c66332 100644 --- a/tests/unit_tests/http_client/test_http_client.py +++ b/tests/unit_tests/http_client/test_http_client.py @@ -519,7 +519,7 @@ def test_request_methods_redirect_failed(self, http_client, method, status): assert len(responses.calls) == 1 @pytest.mark.parametrize('method', ['get', 'post', 'put', 'patch', 'delete']) - def test_request_methods_accept_allow_redirects_false(self, http_client, method): + def test_request_methods_allow_redirects_false_successful(self, http_client, method): # arrange url = http_client._base_url + '/test' responses.add(method.upper(), url, json={}) @@ -544,6 +544,7 @@ def test_request_methods_allow_redirects_true_redirect_failed(self, http_client, getattr(http_client, method)('/test', allow_redirects=True) # assert + assert excinfo.value.code is None assert 'unexpected redirect (307)' in excinfo.value.message assert len(responses.calls) == 1 @@ -559,6 +560,7 @@ def test_handle_error_redirect_failed(): handle_error(response) # assert + assert excinfo.value.code is None assert excinfo.value.message == ( "unexpected redirect (302) to 'https://api.example.com/v1/moved'; redirects are not followed" ) diff --git a/tests/unit_tests/inference_client/test_inference_client.py b/tests/unit_tests/inference_client/test_inference_client.py index 0950d31..78fcc27 100644 --- a/tests/unit_tests/inference_client/test_inference_client.py +++ b/tests/unit_tests/inference_client/test_inference_client.py @@ -29,11 +29,54 @@ VERBS = ['get', 'post', 'put', 'delete', 'patch', 'head', 'options'] +PREDICT_URL = f'{BASE_URL}/predict' +STATUS_URL = 'https://inference.example.com/v1/status/my-deployment' +RESULT_URL = 'https://inference.example.com/v1/result/my-deployment' +REDIRECT_URL = 'https://inference.example.com/v1/moved' +MOVED_URL = f'{BASE_URL}/moved' +EXECUTION_ID = 'exec-1' + +REDIRECT_STATUSES = [301, 302, 303, 307, 308] + + +def _execution(client): + return AsyncInferenceExecution(client, EXECUTION_ID, AsyncStatus.Initialized) + + +# (method, url the call sends to, call) +ENTRY_POINTS = { + 'run_sync': ('POST', PREDICT_URL, lambda c: c.run_sync({'a': 1}, path='predict')), + 'run': ('POST', PREDICT_URL, lambda c: c.run({'a': 1}, path='predict')), + 'run_no_response': ( + 'POST', + PREDICT_URL, + lambda c: c.run({'a': 1}, path='predict', no_response=True), + ), + 'get': ('GET', PREDICT_URL, lambda c: c.get('predict')), + 'post': ('POST', PREDICT_URL, lambda c: c.post('predict', json={'a': 1})), + 'put': ('PUT', PREDICT_URL, lambda c: c.put('predict', json={'a': 1})), + 'delete': ('DELETE', PREDICT_URL, lambda c: c.delete('predict')), + 'patch': ('PATCH', PREDICT_URL, lambda c: c.patch('predict', json={'a': 1})), + 'head': ('HEAD', PREDICT_URL, lambda c: c.head('predict')), + 'options': ('OPTIONS', PREDICT_URL, lambda c: c.options('predict')), + 'health': ('GET', f'{BASE_URL}/health', lambda c: c.health()), + 'status_json': ('GET', STATUS_URL, lambda c: _execution(c).status_json()), + 'result': ('GET', RESULT_URL, lambda c: _execution(c).result()), +} + + @pytest.fixture def inference_client(): return InferenceClient(inference_key='key-123', endpoint_base_url=BASE_URL) +@pytest.fixture +def following_client(): + return InferenceClient( + inference_key='key-123', endpoint_base_url=BASE_URL, follow_redirects=True + ) + + class TestBuildUrl: """`path` is a caller-chosen multi-segment path, but it must stay under the base.""" @@ -243,42 +286,6 @@ def test_the_deployment_name_is_sliced_out_correctly(self, inference_client): assert inference_client.base_domain == 'https://inference.example.com/v1' -PREDICT_URL = f'{BASE_URL}/predict' -STATUS_URL = 'https://inference.example.com/v1/status/my-deployment' -RESULT_URL = 'https://inference.example.com/v1/result/my-deployment' -REDIRECT_URL = 'https://inference.example.com/v1/moved' -MOVED_URL = f'{BASE_URL}/moved' -EXECUTION_ID = 'exec-1' - -REDIRECT_STATUSES = [301, 302, 303, 307, 308] - - -def _execution(client): - return AsyncInferenceExecution(client, EXECUTION_ID, AsyncStatus.Initialized) - - -# (method, url the call sends to, call) -ENTRY_POINTS = { - 'run_sync': ('POST', PREDICT_URL, lambda c: c.run_sync({'a': 1}, path='predict')), - 'run': ('POST', PREDICT_URL, lambda c: c.run({'a': 1}, path='predict')), - 'run_no_response': ( - 'POST', - PREDICT_URL, - lambda c: c.run({'a': 1}, path='predict', no_response=True), - ), - 'get': ('GET', PREDICT_URL, lambda c: c.get('predict')), - 'post': ('POST', PREDICT_URL, lambda c: c.post('predict', json={'a': 1})), - 'put': ('PUT', PREDICT_URL, lambda c: c.put('predict', json={'a': 1})), - 'delete': ('DELETE', PREDICT_URL, lambda c: c.delete('predict')), - 'patch': ('PATCH', PREDICT_URL, lambda c: c.patch('predict', json={'a': 1})), - 'head': ('HEAD', PREDICT_URL, lambda c: c.head('predict')), - 'options': ('OPTIONS', PREDICT_URL, lambda c: c.options('predict')), - 'health': ('GET', f'{BASE_URL}/health', lambda c: c.health()), - 'status_json': ('GET', STATUS_URL, lambda c: _execution(c).status_json()), - 'result': ('GET', RESULT_URL, lambda c: _execution(c).result()), -} - - class TestRedirectResponses: """A 3xx response is reported to the caller as an error.""" @@ -313,7 +320,7 @@ def test_request_redirect_to_another_host_failed(self, inference_client): # assert assert len(responses.calls) == 1 - def test_session_returns_a_redirect_as_is(self, inference_client): + def test_session_redirect_returned_as_is_successful(self, inference_client): # arrange responses.add(responses.GET, PREDICT_URL, status=307, headers={'Location': REDIRECT_URL}) responses.add(responses.GET, REDIRECT_URL, json={}) @@ -371,12 +378,28 @@ def test_request_redirect_to_another_path_has_no_hint_failed(self, inference_cli # assert assert str(excinfo.value).endswith('redirects are not followed') - -@pytest.fixture -def following_client(): - return InferenceClient( - inference_key='key-123', endpoint_base_url=BASE_URL, follow_redirects=True + @pytest.mark.parametrize( + 'location', + [ + 'https://[invalid', + 'https://[invalid/v1/my-deployment/moved', + 'https://inference.example.com:99999/v1/my-deployment/moved', + ], ) + @pytest.mark.parametrize('follow_redirects', [False, True]) + def test_request_redirect_malformed_location_failed(self, follow_redirects, location): + # arrange + client = InferenceClient( + inference_key='key-123', endpoint_base_url=BASE_URL, follow_redirects=follow_redirects + ) + responses.add(responses.GET, PREDICT_URL, status=307, headers={'Location': location}) + + # act + with pytest.raises(InferenceClientError, match=r'redirect \(307\)'): + client.get('predict') + + # assert + assert len(responses.calls) == 1 class TestFollowRedirects: @@ -543,7 +566,7 @@ def test_async_request_redirect_failed(self, following_client, entry_point, stat # assert assert len(responses.calls) == 1 - def test_session_returns_a_redirect_outside_deployment_as_is(self, following_client): + def test_session_redirect_outside_deployment_returned_as_is_successful(self, following_client): # arrange responses.add(responses.GET, PREDICT_URL, status=307, headers={'Location': REDIRECT_URL}) responses.add(responses.GET, REDIRECT_URL, json={}) @@ -629,7 +652,7 @@ def test_result_text_successful(self, inference_client): assert result == {'result': 'plain'} @pytest.mark.parametrize('entry_point', ['status_json', 'result']) - def test_async_requests_use_the_client_timeout(self, inference_client, entry_point): + def test_async_requests_client_timeout_successful(self, inference_client, entry_point): # arrange _, url, call = ENTRY_POINTS[entry_point] responses.add(responses.GET, url, json={'Status': 'Completed'}) @@ -651,7 +674,7 @@ def test_async_requests_failed(self, inference_client, entry_point): call(inference_client) @pytest.mark.parametrize('entry_point', ['status_json', 'result']) - def test_async_requests_timed_out(self, inference_client, entry_point): + def test_async_requests_timeout_failed(self, inference_client, entry_point): # arrange _, url, call = ENTRY_POINTS[entry_point] responses.add(responses.GET, url, body=requests.exceptions.ReadTimeout()) diff --git a/verda/inference_client/_inference_client.py b/verda/inference_client/_inference_client.py index e33d484..0d86780 100644 --- a/verda/inference_client/_inference_client.py +++ b/verda/inference_client/_inference_client.py @@ -151,6 +151,7 @@ def _is_inside_deployment(endpoint_base_url: str, response_url: str, location: s ``//%2E%2E/``. The rest of the target path must pass the same check as ``path`` in ``_build_url``, which also refuses forms such as ``..;/`` and ``..%252f``. The query string is not checked: it does not change the target path. + A ``Location`` that cannot be parsed is not followed. Args: endpoint_base_url: The deployment's base url, without a trailing slash. @@ -160,14 +161,17 @@ def _is_inside_deployment(endpoint_base_url: str, response_url: str, location: s Returns: True if the redirect may be followed. """ - if has_relative_path_segment(urlsplit(location).path): + try: + if has_relative_path_segment(urlsplit(location).path): + return False + target = urljoin(response_url, location) + target_path = urlsplit(target).path + except ValueError: return False - target = urljoin(response_url, location) base_origin = _origin(endpoint_base_url) if base_origin is None or _origin(target) != base_origin: return False deployment_path = urlsplit(endpoint_base_url).path - target_path = urlsplit(target).path if target_path != deployment_path and not target_path.startswith(deployment_path + '/'): return False try: @@ -188,12 +192,16 @@ def _trailing_slash_hint(response_url: str, location: str | None) -> str: location: The ``Location`` header value, if any. Returns: - The hint to append to the error message, or an empty string. + The hint to append to the error message, or an empty string. It is empty if + the ``Location`` cannot be parsed. """ if not location: return '' - request_path = urlsplit(response_url).path - target_path = urlsplit(urljoin(response_url, location)).path + try: + request_path = urlsplit(response_url).path + target_path = urlsplit(urljoin(response_url, location)).path + except ValueError: + return '' stripped = target_path.rstrip('/') if not stripped or request_path.endswith('/') == target_path.endswith('/'): return '' From c1960587e14d14e65f65fb1ccf904288668514b9 Mon Sep 17 00:00:00 2001 From: Dmitrii Iutkin Date: Fri, 30 Oct 2026 11:22:27 +0200 Subject: [PATCH 08/10] Raise only for real redirects, and default the helper timeout A response is now a redirect only if requests would follow it: 301, 302, 303, 307 or 308 with a Location header (Response.is_redirect). Before, every 3xx raised, so a 304 Not Modified became an error. Other 3xx responses go back to the caller, as on master. This applies to InferenceClient._send and to handle_error in the HTTP client. The InferenceClient verb helpers and health() passed timeout_seconds=None, so the request had no timeout. They now use the client timeout_seconds when the caller gives none. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 3 +- CLAUDE.md | 4 +- .../http_client/test_http_client.py | 17 +++++++ .../inference_client/test_inference_client.py | 48 +++++++++++++++++++ verda/http_client/_http_client.py | 12 +++-- verda/inference_client/_inference_client.py | 17 ++++--- 6 files changed, 87 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 37ace10..e819ce4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Security -- `InferenceClient`, `HTTPClient` and the authentication token requests no longer follow HTTP redirects. A 3xx response now raises `InferenceClientError` or `APIException`. The error message includes the `Location` value. +- `InferenceClient`, `HTTPClient` and the authentication token requests no longer follow HTTP redirects. A redirect response now raises `InferenceClientError` or `APIException`. A redirect is a 301, 302, 303, 307 or 308 response with a `Location` header. The error message includes the `Location` value. Other 3xx responses, such as 304, go back to the caller as before. - `InferenceClient` now checks the request `path` with the same rules as the main API client. A path built from untrusted input could inject query parameters or a fragment into a request that carries the inference key, and dot-segment forms such as `..;/` and `..%252f` passed the old check. `path` may now contain only letters, digits, `-._~`, `/` and `:`, and must not contain a `.` or `..` segment. Anything else raises `InferenceClientError` before the request is sent. ### Added @@ -21,6 +21,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - `AsyncInferenceExecution.status_json()` and `result()` now use the client `timeout_seconds` and raise `InferenceClientError` on an HTTP error status. +- The `InferenceClient` methods `get`, `post`, `put`, `delete`, `patch`, `head`, `options` and `health` now use the client `timeout_seconds` when you do not give `timeout_seconds`. Before, these calls had no timeout. To wait longer, set `timeout_seconds` on the client or on the call. - `HTTPClient` methods ignore the `allow_redirects` keyword argument. - **Breaking:** `InferenceClient` no longer accepts a query string or fragment in `path`. Pass query data as `params` instead. For example, replace `client.get('v1/models?limit=10')` with `client.get('v1/models', params={'limit': 10})`, and `client.run_sync(data, path='v1/completions?stream=true')` with `client.run_sync(data, path='v1/completions', params={'stream': 'true'})`. - **Breaking:** `InferenceClient` now accepts only letters, digits, `-._~`, `/` and `:` in `path`. A path with a percent-encoded value (such as `%20` or `%2F`), a space, a non-ASCII character or another character such as `@`, `+`, `,`, `=` or `;` raises `InferenceClientError`. Send such values in the request body or in `params`. diff --git a/CLAUDE.md b/CLAUDE.md index 53149dd..a3fb0a9 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -105,9 +105,9 @@ Reject, do not encode: `requests` decodes `%2E` back to `.` before sending, and All five verbs delegate to a single private `_request`, which is where the url is built and validated. Add new verbs by delegating to it, never by calling `requests` directly. -**Never follow a redirect.** Every request that carries credentials is sent with `allow_redirects=False`, and a 3xx raises: `handle_error` for `HTTPClient` and the auth token calls, and `InferenceClient._send` for inference. `requests` keeps `Authorization` on a same-host redirect, and a 307/308 also keeps the method and body. The `Location` target never went through path validation. Any new send site must pass `allow_redirects=False` and go through the existing error check. Inference requests, including `AsyncInferenceExecution` status and result, go through `InferenceClient._send`, never `_session` directly. +**Never follow a redirect.** Every request that carries credentials is sent with `allow_redirects=False`, and a redirect raises: `handle_error` for `HTTPClient` and the auth token calls, and `InferenceClient._send` for inference. `requests` keeps `Authorization` on a same-host redirect, and a 307/308 also keeps the method and body. The `Location` target never went through path validation. Any new send site must pass `allow_redirects=False` and go through the existing error check. Inference requests, including `AsyncInferenceExecution` status and result, go through `InferenceClient._send`, never `_session` directly. A redirect is `requests.Response.is_redirect`: 301, 302, 303, 307 or 308 with a `Location` header, the same rule `requests` uses to follow. Any other 3xx, such as 304, goes back to the caller. -The one exception is `InferenceClient(follow_redirects=True)`, an opt-in for deployment calls only. `_send` then passes `allow_redirects=True`, and `_RedirectPolicySession.get_redirect_target` reports a target only if `_is_inside_deployment` accepts it: same scheme, host and port as `endpoint_base_url`, a path equal to the deployment path or under `/`, a raw `Location` path that passes `has_relative_path_segment`, and a target path under the deployment path that passes `check_request_path` with `_ALLOWED_IN_INFERENCE_PATH`, the same check as `path`. A rejected target stops `resolve_redirects`, and `_send` raises on the remaining 3xx. The session caps hops at 5. `AsyncInferenceExecution` status and result never follow, because those urls belong to the gateway. Do not add the flag to `HTTPClient` or the auth token calls. +The one exception is `InferenceClient(follow_redirects=True)`, an opt-in for deployment calls only. `_send` then passes `allow_redirects=True`, and `_RedirectPolicySession.get_redirect_target` reports a target only if `_is_inside_deployment` accepts it: same scheme, host and port as `endpoint_base_url`, a path equal to the deployment path or under `/`, a raw `Location` path that passes `has_relative_path_segment`, and a target path under the deployment path that passes `check_request_path` with `_ALLOWED_IN_INFERENCE_PATH`, the same check as `path`. A rejected target stops `resolve_redirects`, and `_send` raises on the remaining redirect. The session caps hops at 5. `AsyncInferenceExecution` status and result never follow, because those urls belong to the gateway. Do not add the flag to `HTTPClient` or the auth token calls. The check is `_encode_path_segment` in `verda/http_client/_http_client.py`; `path_params` is the only supported way to put a caller-supplied value into a path. `_add_base_url` re-asserts the same allowlist on the finished path, as a backstop for a call site that skips `path_params`. diff --git a/tests/unit_tests/http_client/test_http_client.py b/tests/unit_tests/http_client/test_http_client.py index 8c66332..00a075f 100644 --- a/tests/unit_tests/http_client/test_http_client.py +++ b/tests/unit_tests/http_client/test_http_client.py @@ -564,3 +564,20 @@ def test_handle_error_redirect_failed(): assert excinfo.value.message == ( "unexpected redirect (302) to 'https://api.example.com/v1/moved'; redirects are not followed" ) + + +@pytest.mark.parametrize( + ('status', 'headers'), + [(304, {}), (304, {'Location': 'https://api.example.com/v1/moved'}), (300, {}), (307, {})], +) +def test_handle_error_non_redirect_3xx_successful(status, headers): + # arrange + response = requests.Response() + response.status_code = status + response.headers.update(headers) + + # act + handle_error(response) + + # assert + assert response.status_code == status diff --git a/tests/unit_tests/inference_client/test_inference_client.py b/tests/unit_tests/inference_client/test_inference_client.py index 78fcc27..d436d22 100644 --- a/tests/unit_tests/inference_client/test_inference_client.py +++ b/tests/unit_tests/inference_client/test_inference_client.py @@ -378,6 +378,30 @@ def test_request_redirect_to_another_path_has_no_hint_failed(self, inference_cli # assert assert str(excinfo.value).endswith('redirects are not followed') + @pytest.mark.parametrize( + ('status', 'headers'), + [ + (304, {}), + (304, {'Location': MOVED_URL}), + (300, {}), + (307, {}), + ], + ) + @pytest.mark.parametrize('follow_redirects', [False, True]) + def test_request_non_redirect_3xx_successful(self, follow_redirects, status, headers): + # arrange + client = InferenceClient( + inference_key='key-123', endpoint_base_url=BASE_URL, follow_redirects=follow_redirects + ) + responses.add(responses.GET, PREDICT_URL, status=status, headers=headers) + + # act + response = client.get('predict') + + # assert + assert response.status_code == status + assert len(responses.calls) == 1 + @pytest.mark.parametrize( 'location', [ @@ -698,6 +722,30 @@ def test_http_method_helpers_successful(self, inference_client, entry_point): assert response.status_code == 200 assert len(responses.calls) == 1 + @pytest.mark.parametrize( + 'entry_point', ['get', 'post', 'put', 'delete', 'patch', 'head', 'options', 'health'] + ) + def test_http_method_helpers_client_timeout_successful(self, inference_client, entry_point): + # arrange + method, url, call = ENTRY_POINTS[entry_point] + responses.add(method, url, status=200) + + # act + call(inference_client) + + # assert + assert responses.calls[0].request.req_kwargs['timeout'] == inference_client.timeout_seconds + + def test_http_method_helpers_explicit_timeout_successful(self, inference_client): + # arrange + responses.add(responses.GET, PREDICT_URL, status=200) + + # act + inference_client.get('predict', timeout_seconds=7) + + # assert + assert responses.calls[0].request.req_kwargs['timeout'] == 7 + def test_get_with_params_successful(self, inference_client): # arrange responses.add( diff --git a/verda/http_client/_http_client.py b/verda/http_client/_http_client.py index 671f237..4323c10 100644 --- a/verda/http_client/_http_client.py +++ b/verda/http_client/_http_client.py @@ -70,10 +70,12 @@ def _encode_path_segment(value: object) -> str: def handle_error(response: requests.Response) -> None: - """Checks for the response status code and raises an exception if it's 300 or higher. + """Checks the response and raises an exception for a redirect or an error status. - Requests are sent with ``allow_redirects=False``, so a 3xx arrives here unfollowed - and is an error: ``response.ok`` alone would return it to the caller as a success. + Requests are sent with ``allow_redirects=False``, so a redirect arrives here + unfollowed and is an error: ``response.ok`` alone would return it to the caller as + a success. A redirect is what ``requests`` would follow: 301, 302, 303, 307 or 308 + with a ``Location`` header. Any other 3xx, such as 304, is returned unchanged. Args: response: The API call response. @@ -81,8 +83,8 @@ def handle_error(response: requests.Response) -> None: Raises: APIException: An api exception with message and error type code. """ - if 300 <= response.status_code < 400: - location = response.headers.get('Location') + if response.is_redirect: + location = response.headers['Location'] raise APIException( None, f'unexpected redirect ({response.status_code}) to {location!r}; ' diff --git a/verda/inference_client/_inference_client.py b/verda/inference_client/_inference_client.py index 0d86780..f48590e 100644 --- a/verda/inference_client/_inference_client.py +++ b/verda/inference_client/_inference_client.py @@ -405,7 +405,10 @@ def _make_request(self, method: str, path: str, **kwargs) -> requests.Response: Raises: InferenceClientError: If the request fails """ - timeout = kwargs.pop('timeout_seconds', self.timeout_seconds) + # The HTTP verb helpers pass timeout_seconds=None when the caller sets none. + timeout = kwargs.pop('timeout_seconds', None) + if timeout is None: + timeout = self.timeout_seconds return self._send( method, self._build_url(path), @@ -432,7 +435,7 @@ def _send( method and body on a same-host redirect, so a ``Location`` from the response would retarget the request at a path that never went through ``_build_url``. With ``follow_redirects``, the session follows only a target inside the - deployment, and any other 3xx is returned here and raised. + deployment, and any other redirect is returned here and raised. Args: method: HTTP method to use @@ -457,13 +460,15 @@ def _send( allow_redirects=follow_redirects, **kwargs, ) - # raise_for_status() lets a 3xx through. - if 300 <= response.status_code < 400: - location = response.headers.get('Location') + # raise_for_status() lets a 3xx through. is_redirect is the rule requests + # uses to follow: 301, 302, 303, 307 or 308 with a Location header. Any + # other 3xx, such as 304 Not Modified, goes back to the caller. + if response.is_redirect: + location = response.headers['Location'] response.close() reason = ( 'the target is outside the deployment and was not followed' - if follow_redirects and location + if follow_redirects else 'redirects are not followed' ) raise InferenceClientError( From c12e46bf2857d41defed739af49661eed0c1829b Mon Sep 17 00:00:00 2001 From: Dmitrii Iutkin Date: Fri, 30 Oct 2026 11:25:17 +0200 Subject: [PATCH 09/10] Fix the follow_redirects error text and the helper note in CLAUDE.md With follow_redirects on, the error said "the target is outside the deployment" also for a target inside the deployment that failed the path check. It now says that the target failed the follow_redirects check. CLAUDE.md said that has_relative_path_segment is for endpoint_base_url only, but the follow_redirects check also uses it on the raw Location path. The note now names both uses. Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 2 +- examples/containers/calling_the_endpoint_with_redirects.py | 2 +- tests/unit_tests/inference_client/test_inference_client.py | 2 +- verda/inference_client/_inference_client.py | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index a3fb0a9..387c1d2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -113,7 +113,7 @@ The check is `_encode_path_segment` in `verda/http_client/_http_client.py`; `pat The finished-path check is `verda.helpers.check_request_path`. It rejects `?`, `#`, any character outside an allowlist, and `.`/`..` segments. `HTTPClient._add_base_url` calls it with the default allowlist (unreserved set plus `/`). `InferenceClient._build_url` calls it on the caller's multi-segment `path` with `_ALLOWED_IN_INFERENCE_PATH`, which also allows `:` (TensorFlow Serving's `my_model:predict`). Query data for both clients goes in `params`, never in the path. Do not re-implement the check. -`verda.helpers.has_relative_path_segment` strips the query string and decodes escapes and encoded separators. It is for `InferenceClient`'s `endpoint_base_url` only. Do not use it to check a request path. +`verda.helpers.has_relative_path_segment` strips the query string and decodes escapes and encoded separators. It is for two values only: `InferenceClient`'s `endpoint_base_url`, and the raw `Location` path in the `follow_redirects` check (`_is_inside_deployment`), where it runs together with `check_request_path`. Do not use it to check a request path that the SDK builds. ## Code style diff --git a/examples/containers/calling_the_endpoint_with_redirects.py b/examples/containers/calling_the_endpoint_with_redirects.py index a889301..8f0ac0b 100644 --- a/examples/containers/calling_the_endpoint_with_redirects.py +++ b/examples/containers/calling_the_endpoint_with_redirects.py @@ -55,6 +55,6 @@ response = inference_client.run_sync(data=data, path='v1/completions') print(response.output()) except InferenceClientError as e: - # A redirect to a target outside the deployment is not followed. The error + # A redirect that fails the follow_redirects check is not followed. The error # message shows the Location value, and a hint if only a trailing slash differs. print(f'Request failed: {e}') diff --git a/tests/unit_tests/inference_client/test_inference_client.py b/tests/unit_tests/inference_client/test_inference_client.py index d436d22..f0adfec 100644 --- a/tests/unit_tests/inference_client/test_inference_client.py +++ b/tests/unit_tests/inference_client/test_inference_client.py @@ -539,7 +539,7 @@ def test_follow_redirect_outside_deployment_failed(self, following_client, statu following_client.run_sync({'a': 1}, path='predict') # assert - assert 'outside the deployment' in str(excinfo.value) + assert 'failed the follow_redirects check' in str(excinfo.value) assert len(responses.calls) == 1 def test_follow_redirect_stops_at_rejected_hop_failed(self, following_client): diff --git a/verda/inference_client/_inference_client.py b/verda/inference_client/_inference_client.py index f48590e..229aa6a 100644 --- a/verda/inference_client/_inference_client.py +++ b/verda/inference_client/_inference_client.py @@ -467,7 +467,7 @@ def _send( location = response.headers['Location'] response.close() reason = ( - 'the target is outside the deployment and was not followed' + 'the target failed the follow_redirects check and was not followed' if follow_redirects else 'redirects are not followed' ) From fb11d5e03c76c5b1761a0b2afb5342fe23da8d7d Mon Sep 17 00:00:00 2001 From: Dmitrii Iutkin Date: Fri, 30 Oct 2026 11:39:30 +0200 Subject: [PATCH 10/10] Handle failed async jobs and show the error body The gateway returns Status "Error" for a failed job. AsyncStatus had no such value, so status_json() raised ValueError. AsyncStatus now has Error, and the async examples stop polling on it. For a failed job, /result/ returns the deployment's own error status and body. InferenceClientError dropped that body, so the caller could not see why the job failed. An HTTP error now ends with the response body, cut to 500 characters. result() compared Content-Type with 'application/json' exactly, so 'application/json; charset=utf-8' came back as text, and a missing header raised KeyError. It now compares the media type only. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 8 +- .../calling_the_endpoint_asynchronously.py | 10 +- ...g_the_endpoint_with_inference_key_async.py | 12 ++- .../inference_client/test_inference_client.py | 95 +++++++++++++++++++ verda/inference_client/_inference_client.py | 39 +++++++- 5 files changed, 156 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e819ce4..9f71369 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,7 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- `InferenceClient` has a new `follow_redirects` argument. It is off by default. When it is on, the client follows a redirect only if the target is inside the deployment: same scheme, host and port, and a path under `endpoint_base_url` that passes the same rules as `path`. The client follows at most 5 redirects. Any other redirect raises `InferenceClientError`. `AsyncInferenceExecution.status_json()` and `result()` never follow a redirect. +- `InferenceClient` has a new `follow_redirects` argument. It is off by default. When it is on, the client follows a redirect only if the target is inside the deployment. The target must have the same scheme, host and port, and a path under `endpoint_base_url` that passes the same rules as `path`. The client follows at most 5 redirects. Any other redirect raises `InferenceClientError`. `AsyncInferenceExecution.status_json()` and `result()` never follow a redirect. - When a redirect only adds or removes a trailing slash, the `InferenceClientError` message tells you to change `path`. - `InferenceClient.run_sync`, `InferenceClient.run`, `Deployment.run_sync` and `Deployment.run` accept a `params` argument for query string parameters, like the other `InferenceClient` methods. @@ -23,9 +23,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `AsyncInferenceExecution.status_json()` and `result()` now use the client `timeout_seconds` and raise `InferenceClientError` on an HTTP error status. - The `InferenceClient` methods `get`, `post`, `put`, `delete`, `patch`, `head`, `options` and `health` now use the client `timeout_seconds` when you do not give `timeout_seconds`. Before, these calls had no timeout. To wait longer, set `timeout_seconds` on the client or on the call. - `HTTPClient` methods ignore the `allow_redirects` keyword argument. +- An `InferenceClientError` for an HTTP error status now ends with the response body, cut to 500 characters. For a failed async job, `result()` returns the deployment's error status, so the error now shows the deployment's message. - **Breaking:** `InferenceClient` no longer accepts a query string or fragment in `path`. Pass query data as `params` instead. For example, replace `client.get('v1/models?limit=10')` with `client.get('v1/models', params={'limit': 10})`, and `client.run_sync(data, path='v1/completions?stream=true')` with `client.run_sync(data, path='v1/completions', params={'stream': 'true'})`. - **Breaking:** `InferenceClient` now accepts only letters, digits, `-._~`, `/` and `:` in `path`. A path with a percent-encoded value (such as `%20` or `%2F`), a space, a non-ASCII character or another character such as `@`, `+`, `,`, `=` or `;` raises `InferenceClientError`. Send such values in the request body or in `params`. +### Fixed + +- `AsyncInferenceExecution.status_json()` no longer raises `ValueError` for a failed job. `AsyncStatus` has a new value, `Error`. The status JSON has an `Error` field with the reason. +- `AsyncInferenceExecution.result()` now parses a JSON result when the `Content-Type` has parameters, for example `application/json; charset=utf-8`. A result without a `Content-Type` header no longer raises `KeyError`. + ## [1.25.0] - 2026-10-23 ### Deprecated diff --git a/examples/containers/calling_the_endpoint_asynchronously.py b/examples/containers/calling_the_endpoint_asynchronously.py index e8af1ef..af327e3 100644 --- a/examples/containers/calling_the_endpoint_asynchronously.py +++ b/examples/containers/calling_the_endpoint_asynchronously.py @@ -54,7 +54,13 @@ headers=header, ) -while response.status() != AsyncStatus.Completed: +# Poll for status until the job completes or fails +while response.status() not in (AsyncStatus.Completed, AsyncStatus.Error): print(response.status_json()) sleep(1) -print(response.output()) + +if response.status() == AsyncStatus.Error: + # The status json has an Error field with the reason + print(f'Inference failed: {response.status_json()}') +else: + print(response.output()) diff --git a/examples/containers/calling_the_endpoint_with_inference_key_async.py b/examples/containers/calling_the_endpoint_with_inference_key_async.py index 44509f1..c5ce944 100644 --- a/examples/containers/calling_the_endpoint_with_inference_key_async.py +++ b/examples/containers/calling_the_endpoint_with_inference_key_async.py @@ -40,10 +40,14 @@ # Run the request asynchronously using the inference client async_inference_execution = inference_client.run(data=data, path='v1/completions') -# Poll for status until completion -while async_inference_execution.status() != AsyncStatus.Completed: +# Poll for status until the job completes or fails +while async_inference_execution.status() not in (AsyncStatus.Completed, AsyncStatus.Error): print(async_inference_execution.status_json()) sleep(5) -# Print the response -print(async_inference_execution.output()) +if async_inference_execution.status() == AsyncStatus.Error: + # The status json has an Error field with the reason + print(f'Inference failed: {async_inference_execution.status_json()}') +else: + # Print the response + print(async_inference_execution.output()) diff --git a/tests/unit_tests/inference_client/test_inference_client.py b/tests/unit_tests/inference_client/test_inference_client.py index f0adfec..2fcfb42 100644 --- a/tests/unit_tests/inference_client/test_inference_client.py +++ b/tests/unit_tests/inference_client/test_inference_client.py @@ -675,6 +675,101 @@ def test_result_text_successful(self, inference_client): # assert assert result == {'result': 'plain'} + @pytest.mark.parametrize( + 'content_type', ['application/json; charset=utf-8', 'Application/JSON', 'application/json'] + ) + def test_result_json_content_type_successful(self, inference_client, content_type): + # arrange + responses.add(responses.GET, RESULT_URL, body='{"out": 1}', content_type=content_type) + + # act + result = _execution(inference_client).result() + + # assert + assert result == {'out': 1} + + def test_result_without_content_type_successful(self, inference_client): + # arrange + responses.add(responses.GET, RESULT_URL, body='plain', content_type=None) + + # act + result = _execution(inference_client).result() + + # assert + assert 'Content-Type' not in responses.calls[0].response.headers + assert result == {'result': 'plain'} + + def test_status_json_error_successful(self, inference_client): + # arrange + status = { + 'Id': EXECUTION_ID, + 'Status': 'Error', + 'Error': {'Message': 'inference returned non-200 status code 500'}, + } + responses.add(responses.GET, STATUS_URL, json=status) + execution = _execution(inference_client) + + # act + response_json = execution.status_json() + + # assert + assert response_json == status + assert execution.status() == AsyncStatus.Error + + def test_result_of_failed_job_failed(self, inference_client): + # arrange + responses.add( + responses.GET, + RESULT_URL, + status=500, + body='model crashed: out of memory', + content_type='text/plain', + ) + + # act + with pytest.raises(InferenceClientError) as excinfo: + _execution(inference_client).result() + + # assert + assert '500 Server Error' in str(excinfo.value) + assert str(excinfo.value).endswith(': model crashed: out of memory') + + def test_result_before_completion_failed(self, inference_client): + # arrange + responses.add( + responses.GET, RESULT_URL, status=404, json={'msg': 'inference result not found'} + ) + + # act + with pytest.raises(InferenceClientError) as excinfo: + _execution(inference_client).result() + + # assert + assert '404 Client Error' in str(excinfo.value) + assert 'inference result not found' in str(excinfo.value) + + def test_request_long_error_body_failed(self, inference_client): + # arrange + responses.add(responses.POST, PREDICT_URL, status=500, body='x' * 1000) + + # act + with pytest.raises(InferenceClientError) as excinfo: + inference_client.run_sync({'a': 1}, path='predict') + + # assert + assert str(excinfo.value).endswith(': ' + 'x' * 500 + '...') + + def test_request_empty_error_body_failed(self, inference_client): + # arrange + responses.add(responses.POST, PREDICT_URL, status=500, body='') + + # act + with pytest.raises(InferenceClientError) as excinfo: + inference_client.run_sync({'a': 1}, path='predict') + + # assert + assert str(excinfo.value).endswith(f'for url: {PREDICT_URL}') + @pytest.mark.parametrize('entry_point', ['status_json', 'result']) def test_async_requests_client_timeout_successful(self, inference_client, entry_point): # arrange diff --git a/verda/inference_client/_inference_client.py b/verda/inference_client/_inference_client.py index 229aa6a..872439b 100644 --- a/verda/inference_client/_inference_client.py +++ b/verda/inference_client/_inference_client.py @@ -43,6 +43,9 @@ class AsyncStatus(str, Enum): Queue = 'Queue' Inference = 'Inference' Completed = 'Completed' + # The job finished without a result, for example because the deployment + # returned an error status or could not be reached. + Error = 'Error' @dataclass_json(undefined=Undefined.EXCLUDE) @@ -124,6 +127,9 @@ def stream(self, chunk_size: int = 512, as_text: bool = True) -> Generator[Any, yield chunk +# The most characters of a response body that an HTTP error message includes. +_MAX_ERROR_BODY = 500 + # The most redirects a deployment request follows when `follow_redirects` is on. _MAX_REDIRECTS = 5 @@ -181,6 +187,31 @@ def _is_inside_deployment(endpoint_base_url: str, response_url: str, location: s return True +def _error_body(response: requests.Response | None) -> str: + """Return the start of an error response body, to append to the error message. + + For a failed async job, ``/result/`` returns the deployment's own error status and + body. Without the body, the caller cannot see why the inference failed. + + Args: + response: The response with the HTTP error status, if any. + + Returns: + ``': '``, cut to ``_MAX_ERROR_BODY`` characters, or an empty string. + """ + if response is None: + return '' + try: + body = response.text.strip() + except requests.exceptions.RequestException: + return '' + if not body: + return '' + if len(body) > _MAX_ERROR_BODY: + body = body[:_MAX_ERROR_BODY] + '...' + return f': {body}' + + def _trailing_slash_hint(response_url: str, location: str | None) -> str: """Suggest a ``path`` fix when a redirect only adds or removes a trailing slash. @@ -482,6 +513,10 @@ def _send( raise InferenceClientError( f'Request to {path} timed out after {timeout} seconds' ) from e + except requests.exceptions.HTTPError as e: + raise InferenceClientError( + f'Request to {path} failed: {e!s}{_error_body(e.response)}' + ) from e except requests.exceptions.RequestException as e: raise InferenceClientError(f'Request to {path} failed: {e!s}') from e @@ -789,7 +824,9 @@ def result(self) -> dict[str, Any]: ) response = self._get(url) - if response.headers['Content-Type'] == 'application/json': + # The gateway and many servers send 'application/json; charset=utf-8'. + content_type = response.headers.get('Content-Type', '') + if content_type.split(';', 1)[0].strip().lower() == 'application/json': return response.json() else: return {'result': response.text}