Skip to content

Commit 99678db

Browse files
Fix #592
1 parent 581459e commit 99678db

File tree

11 files changed

+152
-34
lines changed

11 files changed

+152
-34
lines changed

CHANGELOG.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
4040
- Correct bug that prevented request body input to be mapped properly to
4141
a `list` using the default logic.
4242
- Upgrade `pytest-asyncio` to the latest version. Fix [#596](https://github.com/Neoteroi/BlackSheep/issues/596).
43+
- Fix a Cython segmentation fault happening when the user defines an exception handler
44+
with a wrong signature ([#592](https://github.com/Neoteroi/BlackSheep/issues/592)),
45+
or that contains a bug and causes exceptions itself.
46+
Replace the Application `exception_handlers` dictionary with a user defined
47+
dictionary that validates values, and change a piece of code that causes
48+
a recursive error when an exception handler itself is buggy.
49+
- Fix `license` field in `pyproject.toml`.
4350

4451
## [2.4.0] - 2025-06-22
4552

blacksheep/baseapp.pxd

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ cdef class BaseApplication:
1212
cdef public bint show_error_details
1313
cdef readonly object router
1414
cdef readonly object logger
15-
cdef public dict exceptions_handlers
15+
cdef public object exceptions_handlers
1616
cpdef object get_http_exception_handler(self, HTTPException http_exception)
1717
cdef object get_exception_handler(self, Exception exception, type stop_at)
1818
cdef bint is_handled_exception(self, Exception exception)

blacksheep/baseapp.py

Lines changed: 39 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,17 @@
11
import http
2+
import inspect
23
import logging
4+
from collections import UserDict
5+
6+
from blacksheep.server.errors import ServerErrorDetailsHandler
37

48
from .contents import Content, TextContent
5-
from .exceptions import HTTPException, InternalServerError, NotFound
9+
from .exceptions import (
10+
HTTPException,
11+
InternalServerError,
12+
InvalidExceptionHandler,
13+
NotFound,
14+
)
615
from .messages import Response
716
from .utils import get_class_instance_hierarchy
817

@@ -12,6 +21,21 @@
1221
ValidationError = None
1322

1423

24+
class ExceptionHandlersDict(UserDict):
25+
26+
def __setitem__(self, key, item) -> None:
27+
if not inspect.iscoroutinefunction(item):
28+
raise InvalidExceptionHandler()
29+
signature = inspect.Signature.from_callable(item)
30+
if len(signature.parameters) != 3 and not any(
31+
param
32+
for param in signature.parameters
33+
if signature.parameters[param].kind == 2
34+
):
35+
raise InvalidExceptionHandler()
36+
return super().__setitem__(key, item)
37+
38+
1539
async def handle_not_found(app, request, http_exception):
1640
return Response(404, content=TextContent("Resource not found"))
1741

@@ -58,9 +82,12 @@ def __init__(self, show_error_details, router):
5882
self.exceptions_handlers = self.init_exceptions_handlers()
5983
self.show_error_details = show_error_details
6084
self.logger = get_logger()
85+
self.server_error_details_handler: ServerErrorDetailsHandler
6186

6287
def init_exceptions_handlers(self):
63-
default_handlers = {404: handle_not_found, 400: handle_bad_request}
88+
default_handlers = ExceptionHandlersDict(
89+
{404: handle_not_found, 400: handle_bad_request}
90+
)
6491
if ValidationError is not None:
6592
default_handlers[ValidationError] = (
6693
_default_pydantic_validation_error_handler
@@ -159,7 +186,16 @@ async def _apply_exception_handler(self, request, exc, exception_handler):
159186
try:
160187
return await exception_handler(self, request, exc)
161188
except Exception as server_ex:
162-
return await self.handle_exception(request, server_ex)
189+
# If the exception happens in the user-defined exception handler,
190+
# we need to fallback to the default handlers.
191+
self.logger.error(
192+
"Unhandled exception in exception_handler: %s",
193+
exception_handler.__name__,
194+
)
195+
if self.show_error_details:
196+
return self.server_error_details_handler.produce_response(request, exc)
197+
198+
return await handle_internal_server_error(self, request, server_ex)
163199

164200
async def handle_http_exception(self, request, http_exception):
165201
exception_handler = self.get_http_exception_handler(http_exception)

blacksheep/baseapp.pyx

Lines changed: 39 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,17 @@
11
import http
2+
import inspect
23
import logging
4+
import sys
5+
from collections import UserDict
36

47
from .contents cimport Content, TextContent
5-
from .exceptions cimport BadRequest, HTTPException, InternalServerError, NotFound
8+
from .exceptions cimport (
9+
BadRequest,
10+
HTTPException,
11+
InternalServerError,
12+
InvalidExceptionHandler,
13+
NotFound,
14+
)
615
from .messages cimport Request, Response
716

817
from .utils import get_class_instance_hierarchy
@@ -14,6 +23,26 @@ except ImportError:
1423
ValidationError = None
1524

1625

26+
_IS_PYTHON_39_OR_OLDER = sys.version_info < (3, 10)
27+
28+
29+
class ExceptionHandlersDict(UserDict):
30+
31+
def __setitem__(self, key, item) -> None:
32+
# In Python 3.9, inspect.iscoroutinefunction does not return the right
33+
# answer for Cython async functions.
34+
if not _IS_PYTHON_39_OR_OLDER and not inspect.iscoroutinefunction(item):
35+
raise InvalidExceptionHandler()
36+
signature = inspect.Signature.from_callable(item)
37+
if len(signature.parameters) != 3 and not any(
38+
param
39+
for param in signature.parameters
40+
if signature.parameters[param].kind == 2
41+
):
42+
raise InvalidExceptionHandler()
43+
return super().__setitem__(key, item)
44+
45+
1746
async def handle_not_found(app, Request request, HTTPException http_exception):
1847
"""Default Not Found handler, returns a simple 404 response."""
1948
return Response(404, content=TextContent("Resource not found"))
@@ -56,10 +85,10 @@ cdef class BaseApplication:
5685
self.logger = get_logger()
5786

5887
def init_exceptions_handlers(self):
59-
default_handlers = {
88+
default_handlers = ExceptionHandlersDict({
6089
404: handle_not_found,
6190
400: handle_bad_request
62-
}
91+
})
6392
if ValidationError is not None:
6493
default_handlers[ValidationError] = _default_pydantic_validation_error_handler
6594
return default_handlers
@@ -178,7 +207,13 @@ cdef class BaseApplication:
178207
try:
179208
return await exception_handler(self, request, exc)
180209
except Exception as server_ex:
181-
return await self.handle_exception(request, server_ex)
210+
# If the exception happens in the user-defined exception handler,
211+
# we need to fallback to the default handlers.
212+
self.logger.error("Unhandled exception in exception_handler: %s", exception_handler.__name__)
213+
if self.show_error_details:
214+
return self.server_error_details_handler.produce_response(request, exc)
215+
216+
return await handle_internal_server_error(self, request, server_ex)
182217

183218
async def handle_http_exception(self, Request request, HTTPException http_exception):
184219
exception_handler = self.get_http_exception_handler(http_exception)

blacksheep/exceptions.pxd

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,3 +35,7 @@ cdef class InvalidOperation(Exception):
3535

3636
cdef class FailedRequestError(HTTPException):
3737
cdef public str data
38+
39+
40+
cdef class InvalidExceptionHandler(Exception):
41+
pass

blacksheep/exceptions.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,3 +77,13 @@ def __init__(self):
7777
super().__init__(
7878
"The message was aborted before the client sent its whole content."
7979
)
80+
81+
82+
class InvalidExceptionHandler(Exception):
83+
84+
def __init__(self) -> None:
85+
super().__init__(
86+
"Invalid Exception handler. Exception handlers must be asynchronous "
87+
"functions with signature: "
88+
"async def handler(app: Application, request: Request, exc: Exception)"
89+
)

blacksheep/exceptions.pyi

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,3 +69,6 @@ class FailedRequestError(HTTPException):
6969
f"The response status code does not indicate success: {status}. Response body: {data}",
7070
)
7171
self.data = data
72+
73+
class InvalidExceptionHandler(Exception):
74+
pass

blacksheep/exceptions.pyx

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,3 +88,13 @@ cdef class MessageAborted(Exception):
8888
super().__init__(
8989
"The message was aborted before the client sent its whole content."
9090
)
91+
92+
93+
cdef class InvalidExceptionHandler(Exception):
94+
95+
def __init__(self) -> None:
96+
super().__init__(
97+
"Invalid Exception handler. Exception handlers must be asynchronous "
98+
"functions with signature: "
99+
"async def handler(app: Application, request: Request, exc: Exception)"
100+
)

issue592.py

Lines changed: 0 additions & 24 deletions
This file was deleted.

pyproject.toml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,8 @@ name = "blacksheep"
77
dynamic = ["version"]
88
authors = [{ name = "Roberto Prevato", email = "[email protected]" }]
99
description = "Fast web framework for Python asyncio"
10-
license = { file = "LICENSE" }
10+
license = "MIT"
11+
license-files = ["LICENSE"]
1112
readme = "README.md"
1213
requires-python = ">=3.8"
1314
classifiers = [

0 commit comments

Comments
 (0)