Skip to content

Commit c13a946

Browse files
authored
🐛 web-server: Handles safely overly long status messages in web server responses (#7760)
1 parent ac9806f commit c13a946

File tree

34 files changed

+184
-113
lines changed

34 files changed

+184
-113
lines changed

packages/service-library/src/servicelib/aiohttp/long_running_tasks/_error_handlers.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,13 @@ async def base_long_running_error_handler(request, handler):
1818
return await handler(request)
1919
except (TaskNotFoundError, TaskNotCompletedError) as exc:
2020
_logger.debug("", exc_info=True)
21-
error_fields = dict(code=exc.code, message=f"{exc}")
21+
error_fields = {"code": exc.code, "message": f"{exc}"}
2222
raise web.HTTPNotFound(
23-
reason=f"{json_dumps(error_fields)}",
23+
text=f"{json_dumps(error_fields)}",
2424
) from exc
2525
except TaskCancelledError as exc:
2626
# NOTE: only use-case would be accessing an already cancelled task
2727
# which should not happen, so we return a conflict
2828
_logger.debug("", exc_info=True)
29-
error_fields = dict(code=exc.code, message=f"{exc}")
30-
raise web.HTTPConflict(reason=f"{json_dumps(error_fields)}") from exc
29+
error_fields = {"code": exc.code, "message": f"{exc}"}
30+
raise web.HTTPConflict(text=f"{json_dumps(error_fields)}") from exc

packages/service-library/src/servicelib/aiohttp/monitoring.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,11 +101,11 @@ async def middleware_handler(request: web.Request, handler: Handler):
101101
log_exception = None
102102
raise resp from exc
103103
except asyncio.CancelledError as exc:
104-
resp = web.HTTPInternalServerError(reason=f"{exc}")
104+
resp = web.HTTPInternalServerError(text=f"{exc}")
105105
log_exception = exc
106106
raise resp from exc
107107
except Exception as exc: # pylint: disable=broad-except
108-
resp = web.HTTPInternalServerError(reason=f"{exc}")
108+
resp = web.HTTPInternalServerError(text=f"{exc}")
109109
resp.__cause__ = exc
110110
log_exception = exc
111111
raise resp from exc

packages/service-library/src/servicelib/aiohttp/requests_validation.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ async def parse_request_body_as(
209209
try:
210210
body = await request.json()
211211
except json.decoder.JSONDecodeError as err:
212-
raise web.HTTPBadRequest(reason=f"Invalid json in body: {err}") from err
212+
raise web.HTTPBadRequest(text=f"Invalid json in body: {err}") from err
213213

214214
if hasattr(model_schema_cls, "model_validate"):
215215
# NOTE: model_schema can be 'list[T]' or 'dict[T]' which raise TypeError

packages/service-library/src/servicelib/aiohttp/rest_middlewares.py

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -81,24 +81,22 @@ async def _middleware_handler(request: web.Request, handler: Handler): # noqa:
8181
return await handler(request)
8282

8383
except web.HTTPError as err:
84-
# TODO: differenciate between server/client error
85-
if not err.reason:
86-
err.set_status(err.status_code, reason="Unexpected error")
8784

8885
err.content_type = MIMETYPE_APPLICATION_JSON
8986

9087
if not err.text or not is_enveloped_from_text(err.text):
91-
error = ErrorGet(
88+
error_message = err.text or err.reason or "Unexpected error"
89+
error_model = ErrorGet(
9290
errors=[
9391
ErrorItemType.from_error(err),
9492
],
9593
status=err.status,
9694
logs=[
97-
LogMessageType(message=err.reason, level="ERROR"),
95+
LogMessageType(message=error_message, level="ERROR"),
9896
],
99-
message=err.reason,
97+
message=error_message,
10098
)
101-
err.text = EnvelopeFactory(error=error).as_text()
99+
err.text = EnvelopeFactory(error=error_model).as_text()
102100

103101
raise
104102

packages/service-library/src/servicelib/aiohttp/rest_responses.py

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,8 +92,7 @@ def create_http_error(
9292
)
9393

9494
return http_error_cls(
95-
# Multiline not allowed in HTTP reason
96-
reason=reason.replace("\n", " ") if reason else None,
95+
reason=safe_status_message(reason),
9796
text=json_dumps(
9897
payload,
9998
),
@@ -129,3 +128,29 @@ def _pred(obj) -> bool:
129128
assert len(http_statuses) == len(found), "No duplicates" # nosec
130129

131130
return http_statuses
131+
132+
133+
def safe_status_message(message: str | None, max_length: int = 50) -> str | None:
134+
"""
135+
Truncates a status-message (i.e. `reason` in HTTP errors) to a maximum length, replacing newlines with spaces.
136+
137+
If the message is longer than max_length, it will be truncated and "..." will be appended.
138+
139+
This prevents issues such as:
140+
- `aiohttp.http_exceptions.LineTooLong`: 400, message: Got more than 8190 bytes when reading Status line is too long.
141+
- Multiline not allowed in HTTP reason attribute (aiohttp now raises ValueError).
142+
143+
See:
144+
- When to use http status and/or text messages https://github.com/ITISFoundation/osparc-simcore/pull/7760
145+
- [RFC 9112, Section 4.1: HTTP/1.1 Message Syntax and Routing](https://datatracker.ietf.org/doc/html/rfc9112#section-4.1) (status line length limits)
146+
- [RFC 9110, Section 15.5: Reason Phrase](https://datatracker.ietf.org/doc/html/rfc9110#section-15.5) (reason phrase definition)
147+
"""
148+
if not message:
149+
return None
150+
151+
flat_message = message.replace("\n", " ")
152+
if len(flat_message) <= max_length:
153+
return flat_message
154+
155+
# Truncate and add ellipsis
156+
return flat_message[: max_length - 3] + "..."

packages/service-library/tests/aiohttp/test_rest_middlewares.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ async def raise_error(_request: web.Request):
112112

113113
@staticmethod
114114
async def raise_error_with_reason(_request: web.Request):
115-
raise web.HTTPNotFound(reason="I did not find it")
115+
raise web.HTTPNotFound(reason="A short phrase")
116116

117117
@staticmethod
118118
async def raise_success(_request: web.Request):

packages/service-library/tests/aiohttp/test_rest_responses.py

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,3 +97,58 @@ def tests_exception_to_response(skip_details: bool, error_code: ErrorCodeStr | N
9797
assert response_json["error"]["message"] == expected_reason
9898
assert response_json["error"]["supportId"] == error_code
9999
assert response_json["error"]["status"] == response.status
100+
101+
102+
@pytest.mark.parametrize(
103+
"input_message, expected_output",
104+
[
105+
(None, None), # None input returns None
106+
("", None), # Empty string returns None
107+
("Simple message", "Simple message"), # Simple message stays the same
108+
(
109+
"Message\nwith\nnewlines",
110+
"Message with newlines",
111+
), # Newlines are replaced with spaces
112+
("A" * 100, "A" * 47 + "..."), # Long message gets truncated with ellipsis
113+
(
114+
"Line1\nLine2\nLine3" + "X" * 100,
115+
"Line1 Line2 Line3" + "X" * 30 + "...",
116+
), # Combined case: newlines and truncation with ellipsis
117+
],
118+
ids=[
119+
"none_input",
120+
"empty_string",
121+
"simple_message",
122+
"newlines_replaced",
123+
"long_message_truncated",
124+
"newlines_and_truncation",
125+
],
126+
)
127+
def test_safe_status_message(input_message: str | None, expected_output: str | None):
128+
from servicelib.aiohttp.rest_responses import safe_status_message
129+
130+
result = safe_status_message(input_message)
131+
assert result == expected_output
132+
133+
# Test with custom max_length
134+
custom_max = 10
135+
result_custom = safe_status_message(input_message, max_length=custom_max)
136+
137+
# Check length constraint is respected
138+
if result_custom is not None:
139+
assert len(result_custom) <= custom_max
140+
# Check that ellipsis is added when truncated
141+
if input_message and len(input_message.replace("\n", " ")) > custom_max:
142+
assert result_custom.endswith("...")
143+
144+
# Verify it can be used in a web response without raising exceptions
145+
try:
146+
# This would fail with long or multiline reasons
147+
if result is not None:
148+
web.Response(reason=result)
149+
150+
# Test with custom length result too
151+
if result_custom is not None:
152+
web.Response(reason=result_custom)
153+
except ValueError:
154+
pytest.fail("safe_status_message result caused an exception in web.Response")

services/web/server/src/simcore_service_webserver/catalog/_catalog_rest_client_service.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ def _handle_client_exceptions(app: web.Application) -> Iterator[ClientSession]:
3939

4040
except ClientResponseError as err:
4141
if err.status == status.HTTP_404_NOT_FOUND:
42-
raise web.HTTPNotFound(reason=MSG_CATALOG_SERVICE_NOT_FOUND)
42+
raise web.HTTPNotFound(text=MSG_CATALOG_SERVICE_NOT_FOUND) from err
4343
raise web.HTTPServiceUnavailable(
4444
reason=MSG_CATALOG_SERVICE_UNAVAILABLE
4545
) from err

services/web/server/src/simcore_service_webserver/exception_handling/_factory.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
from common_library.error_codes import create_error_code
66
from common_library.json_serialization import json_dumps
77
from models_library.rest_error import ErrorGet
8+
from servicelib.aiohttp.rest_responses import safe_status_message
89
from servicelib.aiohttp.web_exceptions_extension import get_all_aiohttp_http_exceptions
910
from servicelib.logging_errors import create_troubleshotting_log_kwargs
1011
from servicelib.status_codes_utils import is_5xx_server_error, is_error
@@ -40,8 +41,7 @@ def create_error_response(error: ErrorGet, status_code: int) -> web.Response:
4041
return web.json_response(
4142
data={"error": error.model_dump(exclude_unset=True, mode="json")},
4243
dumps=json_dumps,
43-
# NOTE: Multiline not allowed in HTTP reason attribute (aiohttp now raises ValueError)
44-
reason=error.message.replace("\n", " ") if error.message else None,
44+
reason=safe_status_message(error.message),
4545
status=status_code,
4646
)
4747

services/web/server/src/simcore_service_webserver/exporter/exceptions.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,5 +6,4 @@ class SDSException(HTTPBadRequest): # pylint: disable=too-many-ancestors
66
"""Basic exception for errors raised inside the module"""
77

88
def __init__(self, message: str):
9-
# Multiline not allowed in HTTP reason attribute
10-
super().__init__(reason=message.replace("\n", " ") if message else None)
9+
super().__init__(text=message)

0 commit comments

Comments
 (0)