diff --git a/src/a2a/utils/proto_utils.py b/src/a2a/utils/proto_utils.py index 1e0003582..f3042e38e 100644 --- a/src/a2a/utils/proto_utils.py +++ b/src/a2a/utils/proto_utils.py @@ -23,7 +23,7 @@ from google.api.field_behavior_pb2 import FieldBehavior, field_behavior from google.protobuf.descriptor import FieldDescriptor -from google.protobuf.json_format import Parse, ParseDict +from google.protobuf.json_format import Parse, ParseDict, ParseError from google.protobuf.message import Message as ProtobufMessage from google.rpc import error_details_pb2 @@ -225,6 +225,10 @@ def parse_params(params: QueryParams, message: ProtobufMessage) -> None: if raw_val is not None: parsed_val: Any = raw_val if field.type == field.TYPE_BOOL and isinstance(raw_val, str): + if raw_val.lower() not in ('true', 'false'): + raise ParseError( + f'Invalid boolean query parameter {k}: expected "true" or "false".' + ) parsed_val = raw_val.lower() == 'true' processed[k] = parsed_val diff --git a/tests/server/routes/test_rest_dispatcher.py b/tests/server/routes/test_rest_dispatcher.py index e0d72c4f2..4865b0cb5 100644 --- a/tests/server/routes/test_rest_dispatcher.py +++ b/tests/server/routes/test_rest_dispatcher.py @@ -3,6 +3,7 @@ from collections.abc import AsyncIterator from unittest.mock import AsyncMock, MagicMock +import httpx import pytest from a2a.server.context import ServerCallContext @@ -11,9 +12,11 @@ from a2a.server.routes.rest_dispatcher import ( RestDispatcher, ) +from a2a.server.routes.rest_routes import create_rest_routes from a2a.types.a2a_pb2 import ( AgentCapabilities, AgentCard, + Artifact, ListTaskPushNotificationConfigsResponse, ListTasksResponse, Message, @@ -23,6 +26,7 @@ from a2a.utils.errors import ( UnsupportedOperationError, ) +from starlette.applications import Starlette from starlette.requests import Request from starlette.responses import JSONResponse @@ -225,6 +229,21 @@ async def test_list_tasks(self, rest_dispatcher_instance, mock_handler): True, id='included', ), + pytest.param( + QueryParams({'includeArtifacts': 'false'}), + False, + id='excluded', + ), + pytest.param( + QueryParams({'includeArtifacts': 'TRUE'}), + True, + id='uppercase-true', + ), + pytest.param( + QueryParams({'includeArtifacts': 'False'}), + False, + id='mixed-case-false', + ), ], ) async def test_list_tasks_artifact_presence( @@ -235,7 +254,12 @@ async def test_list_tasks_artifact_presence( artifacts_expected: bool, ) -> None: mock_handler.on_list_tasks.return_value = ListTasksResponse( - tasks=[Task(id='test_task')] + tasks=[ + Task( + id='test_task', + artifacts=[Artifact(artifact_id='test-artifact')], + ) + ] ) req = make_mock_request(method='GET', query_params=query_params) @@ -243,6 +267,33 @@ async def test_list_tasks_artifact_presence( task = json.loads(response.body)['tasks'][0] assert ('artifacts' in task) is artifacts_expected + if artifacts_expected: + assert task['artifacts'][0]['artifactId'] == 'test-artifact' + + @pytest.mark.parametrize( + 'value', ['garbage', '', '0', '1', ' ', ' true ', 'private-query-value'] + ) + async def test_list_tasks_rejects_invalid_boolean_query( + self, mock_handler: AsyncMock, value: str + ) -> None: + app = Starlette(routes=create_rest_routes(mock_handler)) + async with httpx.AsyncClient( + transport=httpx.ASGITransport(app=app), + base_url='http://test', + trust_env=False, + ) as client: + response = await client.get( + '/tasks', + params={'includeArtifacts': value}, + headers={'a2a-version': '1.0'}, + ) + + assert response.status_code == 400 + assert response.json()['error']['status'] == 'INVALID_ARGUMENT' + assert 'includeArtifacts' in response.json()['error']['message'] + if value.strip(): + assert value not in response.json()['error']['message'] + mock_handler.on_list_tasks.assert_not_awaited() async def test_get_push_notification( self, rest_dispatcher_instance, mock_handler diff --git a/tests/utils/test_proto_utils.py b/tests/utils/test_proto_utils.py index 031aa2e89..6673096e2 100644 --- a/tests/utils/test_proto_utils.py +++ b/tests/utils/test_proto_utils.py @@ -222,6 +222,50 @@ class FakeOldFieldOptional: class TestRestParams: """Unit tests for REST parameter conversion.""" + @pytest.mark.parametrize( + ('values', 'expected'), + [ + ([], None), + (['true'], True), + (['false'], False), + (['TRUE'], True), + (['False'], False), + (['true', 'false'], False), + (['false', 'true'], True), + (['garbage', 'true'], True), + ], + ) + def test_boolean_query_values( + self, values: list[str], expected: bool | None + ) -> None: + message = ListTasksRequest() + + proto_utils.parse_params( + QueryParams([('includeArtifacts', value) for value in values]), + message, + ) + + assert message.HasField('include_artifacts') is (expected is not None) + assert message.include_artifacts is bool(expected) + + @pytest.mark.parametrize( + 'value', ['garbage', '', '0', '1', ' ', ' true ', 'private-query-value'] + ) + @pytest.mark.parametrize('prefix', [[], ['true']]) + def test_invalid_boolean_query_values( + self, value: str, prefix: list[str] + ) -> None: + with pytest.raises(ParseError, match='includeArtifacts') as error: + proto_utils.parse_params( + QueryParams( + [('includeArtifacts', item) for item in [*prefix, value]] + ), + ListTasksRequest(), + ) + + if value.strip(): + assert value not in str(error.value) + def test_rest_params_roundtrip(self): """Test the comprehensive roundtrip conversion for REST parameters."""