Repository navigation
Conversation
|
|
||
| def read(*args, **kwargs): | ||
| self._prepare() | ||
| return getattr(self.__wrapped__, name)(*args, **kwargs) |
There was a problem hiding this comment.
The wrapper looks up the read method again even though value already holds it, adding an unnecessary lookup.
| return getattr(self.__wrapped__, name)(*args, **kwargs) | |
| return value(*args, **kwargs) |
Details
✨ AI Reasoning
__getattr__ already obtains the requested read method and stores it in value. The returned wrapper ignores that result and looks up the same method again, which is an unnecessary extra step.
Reply on this comment to give feedback or ignore the issue.
More info
| if context is not None and ( | ||
| req.content_length or req.environ.get("wsgi.input_terminated") | ||
| ): | ||
| context.set_body_loader(partial(extract_body_from_flask_request, proxy(req))) | ||
| req.environ["aikido_zen.body_context"] = ref(context) | ||
| req.environ["wsgi.input"] = LazyBodyStream( | ||
| req.environ["wsgi.input"], context.load_body | ||
| ) | ||
| req.__dict__["stream"] = LazyBodyStream(req.stream, context.load_body) |
There was a problem hiding this comment.
The body-capture setup is nested under a context-and-body check. This wraps the main path in a precondition and makes the setup harder to scan.
Show fix
| if context is not None and ( | |
| req.content_length or req.environ.get("wsgi.input_terminated") | |
| ): | |
| context.set_body_loader(partial(extract_body_from_flask_request, proxy(req))) | |
| req.environ["aikido_zen.body_context"] = ref(context) | |
| req.environ["wsgi.input"] = LazyBodyStream( | |
| req.environ["wsgi.input"], context.load_body | |
| ) | |
| req.__dict__["stream"] = LazyBodyStream(req.stream, context.load_body) | |
| if context is None or not ( | |
| req.content_length or req.environ.get("wsgi.input_terminated") | |
| ): | |
| return | |
| context.set_body_loader(partial(extract_body_from_flask_request, proxy(req))) | |
| req.environ["aikido_zen.body_context"] = ref(context) | |
| req.environ["wsgi.input"] = LazyBodyStream( | |
| req.environ["wsgi.input"], context.load_body | |
| ) | |
| req.__dict__["stream"] = LazyBodyStream(req.stream, context.load_body) |
Details
✨ AI Reasoning
The setup only applies when a context exists and the request has a body. Keeping that precondition around the whole setup makes the main path unnecessarily nested; a guard would make the applicable path easier to follow.
Reply on this comment to give feedback or ignore the issue.
More info
| if ( | ||
| not has_request_context() | ||
| or request_ctx.request is not request_ref() | ||
| ): | ||
| req = request_ref() | ||
| if req is not None: | ||
| req.close() |
There was a problem hiding this comment.
The load_body cleanup nests the request check beneath the context guard, loader function, and exception-handling block, making this cleanup path difficult to follow.
Show fix
| if ( | |
| not has_request_context() | |
| or request_ctx.request is not request_ref() | |
| ): | |
| req = request_ref() | |
| if req is not None: | |
| req.close() | |
| if has_request_context() and request_ctx.request is request_ref(): | |
| return | |
| req = request_ref() | |
| if req is not None: | |
| req.close() |
Details
✨ AI Reasoning
The deferred body loader is defined inside the context guard, and its cleanup then nests a request-context check inside another condition. This creates a long chain of nested logic in the response handling path, making the cleanup behavior harder to follow.
Reply on this comment to give feedback or ignore the issue.
More info
|
|
||
| def read(*args, **kwargs): | ||
| self._prepare() | ||
| return getattr(self.__wrapped__, name)(*args, **kwargs) |
There was a problem hiding this comment.
🟡 Medium - Lazy capture consumes raw streams before forwarding the caller's read
A Flask handler that reads request.stream, request.input_stream, or environ['wsgi.input'] on a body-bearing request can trigger the new wrapper before receiving the stream bytes. The wrapper runs context.load_body, whose extractor consumes req.form or req.data, and then forwards the same underlying stream at line 34, so the handler receives EOF instead of the request body and can skip its normal parsing or validation.
Show fix
Capture the bytes without consuming the stream that the application is about to read, or buffer and replay the captured bytes through the wrapped stream so the first application read returns the original data. Keep the lazy WAF capture and application stream semantics independent.
More info - Reply on this comment to give feedback or ignore the issue.
| if ( | ||
| not has_request_context() | ||
| or request_ctx.request is not request_ref() | ||
| ): | ||
| req = request_ref() | ||
| if req is not None: | ||
| req.close() |
There was a problem hiding this comment.
🟡 Medium - Deferred multipart parsing leaves uploaded streams open after streaming responses
A multipart upload whose body is first inspected while a streamed response is being consumed can create FileStorage objects after Flask has torn down the original request. The loader runs inside the copied request context, so its cleanup guard skips req.close, and response close only clears the saved context; the upload streams remain referenced by Context.body and can accumulate file descriptors or temporary-file memory across requests.
Show fix
Tie deferred request cleanup to the response close lifecycle: keep the request usable until the streaming/application callbacks finish, then call req.close() in the release callback after the loader's result has been consumed. Ensure this cleanup also runs when the response iterable is closed or abandoned.
More info - Reply on this comment to give feedback or ignore the issue.
Flask currently reads and caches request bodies upfront, even when they remain unused. This adds parsing work and can keep large bodies in memory after the response.
Capture the body when application code reads it, a security check needs it, or API discovery requests it. This avoids unnecessary capture on normal routes and custom error handlers as well as default errors. Raw stream reads, forms, uploads, and streamed responses remain covered.
In a local Gunicorn test, 60 POSTs with 24 MiB bodies to a custom 404 handler that ignores the body reduced peak worker anonymous memory from 584 MiB to 32 MiB (about 94% lower).
Local request-capture CPU benchmarks, two runs per case, with reporting and discovery disabled:
Unused-body cases save roughly 76–90% CPU. Small JSON reads add about 20–33 microseconds per request in this benchmark.