-
Notifications
You must be signed in to change notification settings - Fork 12
add uvicorn context-creation support when run with gUnicorn #576
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
bitterpanda63
wants to merge
49
commits into
main
Choose a base branch
from
add-django-asgi-support
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
49 commits
Select commit
Hold shift + click to select a range
a2d632e
Create general use asgi middleware
bitterpanda63 d708e59
Update quart.py to start using this internal middleware
bitterpanda63 44b28b0
asgi_middleware, write response
bitterpanda63 dbdd8f7
quart: remove code that otherwise would write the resposne
bitterpanda63 fc3c5fd
Fix asgi_middleware bugs
bitterpanda63 3e17be0
quart instead of __call__, use the asgi_app hook
bitterpanda63 fa05e8d
create a run_with_intercepts for asgi middleware
bitterpanda63 d86a170
Fix interceptor (add comments), and fix lint issue
bitterpanda63 706bde2
Fix asgi_middleware.py (use message as default name, explain)
bitterpanda63 bc6b241
quart.py : remove status code handler
bitterpanda63 c4d1a53
Linting fix: add extra line for quart.py
bitterpanda63 b02ecca
remove status_code from quart.py docstring
bitterpanda63 840b0c5
Rename InterceptorSkeletonClass to InterceptorSkeleton
bitterpanda63 8448c56
asgi_middleware: add support for legacy
bitterpanda63 a9818dc
quart: support both legacy and non-legacy
bitterpanda63 a33f58d
hypercorn_asgi
bitterpanda63 4959447
hypercorn asgi sample app (WIP)
bitterpanda63 5e3587a
add cli tool
bitterpanda63 7f97945
pythonpath try outs
bitterpanda63 8b80276
Revert "pythonpath try outs"
bitterpanda63 0e1097a
create django app (basics)
bitterpanda63 f6ea71f
add poetry and makefile basics
bitterpanda63 924d1fc
add a small readme and update the main readme
bitterpanda63 a0ad361
add aux files for django app & gunicorn
bitterpanda63 94dd5f6
switches to psycopg for async
bitterpanda63 47038d8
delete unuseful wsgi.py file
bitterpanda63 3d2899a
Cleanup a bunch of logs & files
bitterpanda63 cb5972d
cleanup settings.py
bitterpanda63 e306e78
cleanup sample-django-asgi-uvicorn-app/urls.py
bitterpanda63 c8292ec
django asgi sample app: full async
bitterpanda63 0b0d2de
Update comments in __init__.py imports
bitterpanda63 a2a42f1
Add gunicorn.py Worker.__init__ source, testing for now
bitterpanda63 e5362f3
patch uvicorn instead of gunicron
bitterpanda63 5acd6dc
Merge branch 'general-use-asgi-middleware' into add-django-asgi-support
bitterpanda63 9f2323d
add uvicorn to asgi middleware
bitterpanda63 0b51773
delete quart-postgres-hypercorn
bitterpanda63 96d5f5c
delete cli try outs
bitterpanda63 8e7db0d
Delete aikido_zen/sources/hypercorn_asgi.py
bitterpanda63 4226b9b
Apply suggestions from code review
bitterpanda63 2b1714a
Revert quart changes
bitterpanda63 f8a3ddc
Spacing fix
bitterpanda63 bbd61ae
Docs update
bitterpanda63 1a40cfa
Add a limited support
bitterpanda63 596709f
specify ASGI so people don't get confused
bitterpanda63 a5f1bf6
quart.py: Simplify by using the send_status_code_and_text from asgi fβ¦
bitterpanda63 c0bdc78
Add test cases for asgi_middleware
bitterpanda63 4836a3c
add an e2e test case for django asgi uvicorn
bitterpanda63 af0b50c
psycopg: add async support
bitterpanda63 cde98e8
run_init_stage: Does not get ASGI contexts
bitterpanda63 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,93 @@ | ||
| import inspect | ||
| from aikido_zen.context import Context | ||
| from aikido_zen.helpers.get_argument import get_argument | ||
| from aikido_zen.sinks import before_async, patch_function | ||
| from aikido_zen.sources.functions.request_handler import request_handler, post_response | ||
| from aikido_zen.thread.thread_cache import get_cache | ||
|
|
||
|
|
||
| class InternalASGIMiddleware: | ||
| def __init__(self, app, source: str): | ||
| self.client_app = app | ||
| self.source = source | ||
|
|
||
| async def __call__(self, scope, receive, send): | ||
| if not scope or scope.get("type") != "http": | ||
| # Zen checks requests coming into HTTP(S) server, ignore other requests (like ws) | ||
| return await self.continue_app(scope, receive, send) | ||
|
|
||
| context = Context(req=scope, source=self.source) | ||
|
|
||
| process_cache = get_cache() | ||
| if process_cache and process_cache.is_bypassed_ip(context.remote_address): | ||
| # IP address is bypassed, for simplicity we do not set a context, | ||
| # and we do not do any further handling of the request. | ||
| return await self.continue_app(scope, receive, send) | ||
|
|
||
| context.set_as_current_context() | ||
| if process_cache: | ||
| # Since this SHOULD be the highest level of the apps we wrap, this is the safest place | ||
| # to increment total hits. | ||
| process_cache.stats.increment_total_hits() | ||
|
|
||
| intercept_response = request_handler(stage="pre_response") | ||
| if intercept_response: | ||
| # The request has already been blocked (e.g. IP is on blocklist) | ||
| return await send_status_code_and_text(send, intercept_response) | ||
|
|
||
| return await self.run_with_intercepts(scope, receive, send) | ||
|
|
||
| async def run_with_intercepts(self, scope, receive, send): | ||
| # We use a skeleton class so we can use patch_function (and the logic already defined in @before_async) | ||
| class InterceptorSkeleton: | ||
| @staticmethod | ||
| async def send(*args, **kwargs): | ||
| return await send(*args, **kwargs) | ||
|
|
||
| patch_function(InterceptorSkeleton, "send", send_interceptor) | ||
|
|
||
| return await self.continue_app(scope, receive, InterceptorSkeleton.send) | ||
|
|
||
| async def continue_app(self, scope, receive, send): | ||
| client_app_parameters = len(inspect.signature(self.client_app).parameters) | ||
| if client_app_parameters == 2: | ||
| # This is possible if the app is still using ASGI v2.0 | ||
| # See https://asgi.readthedocs.io/en/latest/specs/main.html#legacy-applications | ||
| # client_app = coroutine application_instance(receive, send) | ||
| await self.client_app(receive, send) | ||
| else: | ||
| # client_app = coroutine application(scope, receive, send) | ||
| await self.client_app(scope, receive, send) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no need for return here?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. wdym, returning the value? not done for ASGI apps |
||
|
|
||
|
|
||
| async def send_status_code_and_text(send, pre_response): | ||
| await send( | ||
| { | ||
| "type": "http.response.start", | ||
| "status": pre_response[1], | ||
| "headers": [(b"content-type", b"text/plain")], | ||
| } | ||
| ) | ||
| await send( | ||
| { | ||
| "type": "http.response.body", | ||
| "body": pre_response[0].encode("utf-8"), | ||
| "more_body": False, | ||
| } | ||
| ) | ||
|
|
||
|
|
||
| @before_async | ||
| async def send_interceptor(func, instance, args, kwargs): | ||
| # There is no name for the send() comment in the standard, it really depends (quart uses message) | ||
| event = get_argument(args, kwargs, 0, name="message") | ||
|
|
||
| # https://asgi.readthedocs.io/en/latest/specs/www.html#response-start-send-event | ||
| if not event or "http.response.start" not in event.get("type", ""): | ||
| # If the event is not of type http.response.start it won't contain the status code. | ||
| # And this event is required before sending over a body (so even 200 status codes are intercepted). | ||
| return | ||
|
|
||
| if "status" in event: | ||
| # Handle post response logic (attack waves, route reporting, ...) | ||
| post_response(status_code=int(event.get("status"))) | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Docstring in patch_async only restates that it patches psycopg.cursor_async. Replace with the rationale for the async-specific patch or remove the redundant comment.
Details
β¨ AI Reasoning
βAn added docstring for the newly introduced async patch function merely restates that it patches the psycopg.cursor_async module and that it's similar to the normal patch. This repeats what the code (registering AsyncCursor.copy/execute/executemany) already shows and doesn't explain why the separate async patch is needed or any design rationale. A 'why' comment or removal would be more valuable.
π§ How do I fix it?
Write comments that explain the purpose, reasoning, or business logic behind the code using words like 'because', 'so that', or 'in order to'.
Reply
@AikidoSec feedback: [FEEDBACK]to get better review comments in the future.Reply
@AikidoSec ignore: [REASON]to ignore this issue.More info