Skip to content

Commit b74e3da

Browse files
authored
fix(appsec): catch correctly BlockingException aggregated in ExceptionGroup (#14250)
## Description When an Appsec `BlockingException` is raised within a fastapi middleware, it can be aggregated and nested in a `BaseExceptionGroup` multiple levels deep. The behaviour occurs when using http middlewares. ## Checklist - [x] PR author has checked that all the criteria below are met - The PR description includes an overview of the change - The PR description articulates the motivation for the change - The change includes tests OR the PR description describes a testing strategy - The PR description notes risks associated with the change, if any - Newly-added code is easy to change - The change follows the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] Reviewer has checked that all the criteria below are met - Title is accurate - All changes are related to the pull request's stated goal - Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - Testing strategy adequately addresses listed risks - Newly-added code is easy to change - Release note makes sense to a user of the library - If necessary, author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment - Backport labels are set in a manner that is consistent with the [release branch maintenance policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)
1 parent f82c49a commit b74e3da

File tree

4 files changed

+44
-5
lines changed

4 files changed

+44
-5
lines changed

ddtrace/contrib/internal/asgi/middleware.py

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
from ddtrace.ext.net import TARGET_HOST
2323
from ddtrace.internal import core
2424
from ddtrace.internal._exceptions import BlockingException
25+
from ddtrace.internal._exceptions import find_exception
2526
from ddtrace.internal.compat import is_valid_ip
2627
from ddtrace.internal.constants import COMPONENT
2728
from ddtrace.internal.constants import SAMPLING_DECISION_MAKER_INHERITED
@@ -498,11 +499,9 @@ async def wrapped_blocked_send(message: Mapping[str, Any]):
498499
raise
499500
except BaseException as exception:
500501
# managing python 3.11+ BaseExceptionGroup with compatible code for 3.10 and below
501-
if exception.__class__.__name__ == "BaseExceptionGroup":
502-
for exc in exception.exceptions:
503-
if isinstance(exc, BlockingException):
504-
set_blocked(exc.args[0])
505-
return await _blocked_asgi_app(scope, receive, wrapped_blocked_send)
502+
if exc := find_exception(exception, BlockingException):
503+
set_blocked(exc.args[0])
504+
return await _blocked_asgi_app(scope, receive, wrapped_blocked_send)
506505
raise
507506
finally:
508507
core.dispatch("web.request.final_tags", (span,))

ddtrace/internal/_exceptions.py

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,29 @@
1+
from typing import Optional
2+
from typing import Type
3+
from typing import TypeVar
4+
5+
16
class BlockingException(BaseException):
27
"""
38
Exception raised when a request is blocked by ASM
49
It derives from BaseException to avoid being caught by the general Exception handler
510
"""
11+
12+
13+
E = TypeVar("E", bound=BaseException)
14+
15+
16+
def find_exception(
17+
exc: BaseException,
18+
exception_type: Type[E],
19+
) -> Optional[E]:
20+
"""Traverse an exception and its children to find the first occurrence of a specific exception type."""
21+
if isinstance(exc, exception_type):
22+
return exc
23+
# The check matches both native Python3.11+ and `exceptiongroup` compatibility package versions of ExceptionGroup
24+
if exc.__class__.__name__ in ("BaseExceptionGroup", "ExceptionGroup") and hasattr(exc, "exceptions"):
25+
for sub_exc in exc.exceptions:
26+
found = find_exception(sub_exc, exception_type)
27+
if found:
28+
return found
29+
return None
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
fixes:
3+
- |
4+
AAP: resolves a bug where ASGI middleware would not catch the BlockingException raised by AAP because it was aggregated in an ExceptionGroup

tests/appsec/contrib_appsec/fastapi_app/app.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,18 @@ class User(BaseModel):
3939
def get_app():
4040
app = FastAPI()
4141

42+
@app.middleware("http")
43+
async def passthrough_middleware(request: Request, call_next):
44+
"""Middleware to test BlockingException nesting in ExceptionGroups (or BaseExceptionGroups)
45+
46+
With middlewares, the BlockingException can become nested multiple levels deep inside
47+
an ExceptionGroup (or BaseExceptionGroup). The nesting depends the version of FastAPI
48+
and AnyIO used, as well as the version of python.
49+
By adding this empty middleware, we ensure that the BlockingException is catched
50+
no matter how deep the ExceptionGroup is nested or else the contrib tests fail.
51+
"""
52+
return await call_next(request)
53+
4254
@app.get("/")
4355
@app.post("/")
4456
@app.options("/")

0 commit comments

Comments
 (0)