-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add .prof File Download Support to Profiling Panel #2146
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -142,7 +142,20 @@ def _last_executed_query(self, sql, params): | |
# process during the .last_executed_query() call. | ||
self.db._djdt_logger = None | ||
try: | ||
return self.db.ops.last_executed_query(self.cursor, sql, params) | ||
# Handle executemany: take the first set of parameters for formatting | ||
if ( | ||
isinstance(params, (list, tuple)) | ||
and len(params) > 0 | ||
and isinstance(params[0], (list, tuple)) | ||
): | ||
sample_params = params[0] | ||
else: | ||
sample_params = params | ||
|
||
try: | ||
return self.db.ops.last_executed_query(self.cursor, sql, sample_params) | ||
except Exception: | ||
return sql | ||
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. This looks like an unrelated change? |
||
finally: | ||
self.db._djdt_logger = self.logger | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,13 @@ | ||
{% load i18n %} | ||
|
||
{% if prof_file_path %} | ||
<div style="margin-bottom: 10px;"> | ||
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. We do not have any inline styles in templates until now, if we need styles add them to the |
||
<a href="{% url 'debug_toolbar_download_prof_file' %}?path={{ prof_file_path|urlencode }}" class="djDebugButton"> | ||
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. The toolbar uses the |
||
Download .prof file | ||
</a> | ||
</div> | ||
{% endif %} | ||
|
||
<table> | ||
<thead> | ||
<tr> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,14 @@ | ||
from debug_toolbar import APP_NAME | ||
from django.urls import path | ||
|
||
from debug_toolbar import APP_NAME, views as debug_toolbar_views | ||
from debug_toolbar.toolbar import DebugToolbar | ||
|
||
app_name = APP_NAME | ||
urlpatterns = DebugToolbar.get_urls() | ||
|
||
urlpatterns = DebugToolbar.get_urls() + [ | ||
path( | ||
"download_prof_file/", | ||
debug_toolbar_views.download_prof_file, | ||
name="debug_toolbar_download_prof_file", | ||
), | ||
] |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,9 @@ | ||
from django.http import JsonResponse | ||
import os | ||
|
||
from django.http import FileResponse, Http404, JsonResponse | ||
from django.utils.html import escape | ||
from django.utils.translation import gettext as _ | ||
from django.views.decorators.http import require_GET | ||
|
||
from debug_toolbar._compat import login_not_required | ||
from debug_toolbar.decorators import render_with_toolbar_language, require_show_toolbar | ||
|
@@ -25,3 +28,20 @@ def render_panel(request): | |
content = panel.content | ||
scripts = panel.scripts | ||
return JsonResponse({"content": content, "scripts": scripts}) | ||
|
||
|
||
@require_GET | ||
def download_prof_file(request): | ||
file_path = request.GET.get("path") | ||
print("Serving .prof file:", file_path) | ||
if not file_path or not os.path.exists(file_path): | ||
print("File does not exist:", file_path) | ||
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. This can trivially be exploited by using something like
This view serves arbitrary files which are readable by the webserver process: Either you have to use signing ( |
||
raise Http404("File not found.") | ||
|
||
response = FileResponse( | ||
open(file_path, "rb"), content_type="application/octet-stream" | ||
) | ||
response["Content-Disposition"] = ( | ||
f'attachment; filename="{os.path.basename(file_path)}"' | ||
) | ||
return response |
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.
_get_candidate_names()
isn't documented? I think we should only enable this feature when users provide a path where we can store profiles, because we have to ensure we're only serving files from this very path and not arbitrary files.