Skip to content

Commit 97b001d

Browse files
fix(appsec): catch correctly BlockingException aggregated in ExceptionGroup [backport 3.11] (#14269)
Backport b74e3da from #14250 to 3.11. ## 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) Co-authored-by: Christophe Papazian <[email protected]> Co-authored-by: Christophe Papazian <[email protected]>
1 parent 647e5e8 commit 97b001d

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
@@ -15,6 +15,7 @@
1515
from ddtrace.ext import http
1616
from ddtrace.internal import core
1717
from ddtrace.internal._exceptions import BlockingException
18+
from ddtrace.internal._exceptions import find_exception
1819
from ddtrace.internal.compat import is_valid_ip
1920
from ddtrace.internal.constants import COMPONENT
2021
from ddtrace.internal.logger import get_logger
@@ -320,11 +321,9 @@ async def wrapped_blocked_send(message):
320321
raise
321322
except BaseException as exception:
322323
# managing python 3.11+ BaseExceptionGroup with compatible code for 3.10 and below
323-
if exception.__class__.__name__ == "BaseExceptionGroup":
324-
for exc in exception.exceptions:
325-
if isinstance(exc, BlockingException):
326-
set_blocked(exc.args[0])
327-
return await _blocked_asgi_app(scope, receive, wrapped_blocked_send)
324+
if exc := find_exception(exception, BlockingException):
325+
set_blocked(exc.args[0])
326+
return await _blocked_asgi_app(scope, receive, wrapped_blocked_send)
328327
raise
329328
finally:
330329
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)