process PR feedback

Author: pi-sigma

.github/workflows/ci.yml

@@ -15,11 +15,8 @@ jobs:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        python: ["3.7", "3.8", "3.9", "3.10"]
+        python: ["3.8", "3.9", "3.10"]
         django: ["3.2", "4.1"]
-        exclude:
-          - python: "3.7"
-            django: "4.1"
 
     name: Run the test suite (Python ${{ matrix.python }}, Django ${{ matrix.django }})
 

README.rst

@@ -97,8 +97,16 @@ To use this with your project you need to follow these steps:
         }
 
         LOG_OUTGOING_REQUESTS_DB_SAVE = True # save logs enabled/disabled based on the boolean value
-        LOG_OUTGOING_REQUESTS_SAVE_BODY = True # save request/response body
-        LOG_OUTGOING_REQUESTS_LOG_BODY_TO_STDOUT = True # log request/response body to STDOUT
+        LOG_OUTGOING_REQUESTS_DB_SAVE_BODY = True # save request/response body
+        LOG_OUTGOING_REQUESTS_EMIT_BODY = True # log request/response body
+        LOG_OUTGOING_REQUESTS_CONTENT_TYPES = [
+                "text/*",
+                "application/json",
+                "application/xml",
+                "application/soap+xml",
+        ] # save request/response bodies with matching content type
+        LOG_OUTGOING_REQUESTS_MAX_CONTENT_LENGTH = 524_288  # maximal size (in bytes) for the request/response body
+        LOG_OUTGOING_REQUESTS_LOG_BODY_TO_STDOUT = True
 
 
 #.  Run the migrations
@@ -115,8 +123,9 @@ To use this with your project you need to follow these steps:
         res = requests.get("https://httpbin.org/json")
         print(res.json())
 
-#.  Check stdout for the printable output, and navigate to ``/admin/log_outgoing_requests/outgoingrequestslog/`` to see 
-    the saved log records. The settings for saving logs can by overridden under ``/admin/log_outgoing_requests/outgoingrequestslogconfig/``.
+#.  Check stdout for the printable output, and navigate to ``Admin > Miscellaneous > Outgoing Requests Logs``
+    to see the saved log records. In order to override the settings for saving logs, navigate to
+    ``Admin > Miscellaneous > Outgoing Requests Log Configuration``.
 
 
 Local development

docs/quickstart.rst

@@ -58,8 +58,16 @@ Installation
         }
 
         LOG_OUTGOING_REQUESTS_DB_SAVE = True # save logs enabled/disabled based on the boolean value
-        LOG_OUTGOING_REQUESTS_SAVE_BODY = True # save request/response body
-        LOG_OUTGOING_REQUESTS_LOG_BODY_TO_STDOUT = True # log request/response body to STDOUT
+        LOG_OUTGOING_REQUESTS_DB_SAVE_BODY = True # save request/response body
+        LOG_OUTGOING_REQUESTS_EMIT_BODY = True # log request/response body
+        LOG_OUTGOING_REQUESTS_CONTENT_TYPES = [
+                "text/*",
+                "application/json",
+                "application/xml",
+                "application/soap+xml",
+        ] # save request/response bodies with matching content type
+        LOG_OUTGOING_REQUESTS_MAX_CONTENT_LENGTH = 524_288  # maximal size (in bytes) for the request/response body
+        LOG_OUTGOING_REQUESTS_LOG_BODY_TO_STDOUT = True
 
 
 #.  Run ``python manage.py migrate`` to create the necessary database tables.

log_outgoing_requests/admin.py

@@ -1,16 +1,14 @@
+from django import forms
+from django.conf import settings
 from django.contrib import admin
+from django.shortcuts import get_object_or_404
 from django.utils.translation import gettext as _
 
-from solo.admin import SingletonModelAdmin
+from solo.admin import SingletonModelAdmin  # type: ignore
 
 from .models import OutgoingRequestsLog, OutgoingRequestsLogConfig
 
 
-@admin.display(description="Response body")
-def response_body(obj):
-    return f"{obj}".upper()
-
-
 @admin.register(OutgoingRequestsLog)
 class OutgoingRequestsLogAdmin(admin.ModelAdmin):
     fields = (
@@ -26,8 +24,6 @@ class OutgoingRequestsLogAdmin(admin.ModelAdmin):
         "res_content_type",
         "req_headers",
         "res_headers",
-        "req_body",
-        "res_body",
         "trace",
     )
     readonly_fields = fields
@@ -40,10 +36,11 @@ class OutgoingRequestsLogAdmin(admin.ModelAdmin):
         "response_ms",
         "timestamp",
     )
-    list_filter = ("method", "status_code", "hostname", "timestamp")
+    list_filter = ("method", "timestamp", "status_code", "hostname")
     search_fields = ("url", "params", "hostname")
     date_hierarchy = "timestamp"
     show_full_result_count = False
+    change_form_template = "log_outgoing_requests/change_form.html"
 
     def has_add_permission(self, request):
         return False
@@ -54,21 +51,41 @@ def has_change_permission(self, request, obj=None):
     def query_params(self, obj):
         return obj.query_params
 
-    query_params.short_description = _("Query parameters")
+    def change_view(self, request, object_id, extra_context=None):
+        """
+        Add log object to to context for use in template.
+        """
+        log = get_object_or_404(OutgoingRequestsLog, id=object_id)
+
+        extra_context = extra_context or {}
+        extra_context["log"] = log
+
+        return super().change_view(request, object_id, extra_context=extra_context)
+
+    query_params.short_description = _("Query parameters")  # type: ignore
 
     class Media:
         css = {
             "all": ("log_outgoing_requests/css/admin.css",),
         }
 
 
+class ConfigAdminForm(forms.ModelForm):
+    class Meta:
+        model = OutgoingRequestsLogConfig
+        fields = "__all__"
+        widgets = {"allowed_content_types": forms.CheckboxSelectMultiple}
+        help_texts = {
+            "save_to_db": _(
+                "Whether request logs should be saved to the database (default: {default})."
+            ).format(default=settings.LOG_OUTGOING_REQUESTS_DB_SAVE),
+            "save_body": _(
+                "Wheter the body of the request and response should be logged (default: "
+                "{default})."
+            ).format(default=settings.LOG_OUTGOING_REQUESTS_DB_SAVE_BODY),
+        }
+
+
 @admin.register(OutgoingRequestsLogConfig)
 class OutgoingRequestsLogConfigAdmin(SingletonModelAdmin):
-    fields = (
-        "save_to_db",
-        "save_body",
-    )
-    list_display = (
-        "save_to_db",
-        "save_body",
-    )
+    form = ConfigAdminForm

log_outgoing_requests/constants.py

@@ -0,0 +1,8 @@
+from django.db import models
+from django.utils.translation import gettext_lazy as _
+
+
+class SaveLogsChoice(models.TextChoices):
+    use_default = "use_default", _("Use default")
+    yes = "yes", _("Yes")
+    no = "no", _("No")

log_outgoing_requests/datastructures.py

@@ -0,0 +1,18 @@
+"""
+Datastructure(s) for use in settings.py
+
+Note: do not place any Django-specific imports in this file, as
+it must be imported in settings.py.
+"""
+
+from dataclasses import dataclass
+
+
+@dataclass
+class ContentType:
+    """
+    Data class for keeping track of content types and associated default encodings
+    """
+
+    pattern: str
+    default_encoding: str

log_outgoing_requests/formatters.py

@@ -8,32 +8,35 @@ class HttpFormatter(logging.Formatter):
     def _formatHeaders(self, d):
         return "\n".join(f"{k}: {v}" for k, v in d.items())
 
-    def _formatBody(self, content: dict, request_or_response: str) -> str:
-        if settings.LOG_OUTGOING_REQUESTS_LOG_BODY_TO_STDOUT:
+    def _formatBody(self, content: str, request_or_response: str) -> str:
+        if settings.LOG_OUTGOING_REQUESTS_EMIT_BODY:
             return f"\n{request_or_response} body:\n{content}"
         return ""
 
     def formatMessage(self, record):
         result = super().formatMessage(record)
-        if record.name == "requests":
-            result += textwrap.dedent(
-                """
-                ---------------- request ----------------
-                {req.method} {req.url}
-                {reqhdrs} {request_body}
 
-                ---------------- response ----------------
-                {res.status_code} {res.reason} {res.url}
-                {reshdrs} {response_body}
+        if record.name != "requests":
+            return result
 
+        result += textwrap.dedent(
             """
-            ).format(
-                req=record.req,
-                res=record.res,
-                reqhdrs=self._formatHeaders(record.req.headers),
-                reshdrs=self._formatHeaders(record.res.headers),
-                request_body=self._formatBody(record.req.body, "Request"),
-                response_body=self._formatBody(record.res.json(), "Response"),
-            )
+            ---------------- request ----------------
+            {req.method} {req.url}
+            {reqhdrs} {request_body}
+
+            ---------------- response ----------------
+            {res.status_code} {res.reason} {res.url}
+            {reshdrs} {response_body}
+
+        """
+        ).format(
+            req=record.req,
+            res=record.res,
+            reqhdrs=self._formatHeaders(record.req.headers),
+            reshdrs=self._formatHeaders(record.res.headers),
+            request_body=self._formatBody(record.req.body, "Request"),
+            response_body=self._formatBody(record.res.content, "Response"),
+        )
 
         return result

log_outgoing_requests/handlers.py

@@ -2,70 +2,71 @@
 import traceback
 from urllib.parse import urlparse
 
-from django.conf import settings
-
-ALLOWED_CONTENT_TYPES = [
-    "application/json",
-    "multipart/form-data",
-    "text/html",
-    "text/plain",
-    "",
-    None,
-]
+from .utils import (
+    check_content_length,
+    check_content_type,
+    get_default_encoding,
+    parse_content_type_header,
+)
 
 
 class DatabaseOutgoingRequestsHandler(logging.Handler):
     def emit(self, record):
-        from .models import OutgoingRequestsLogConfig
+        from .models import OutgoingRequestsLog, OutgoingRequestsLogConfig
 
         config = OutgoingRequestsLogConfig.get_solo()
 
-        if config.save_to_db or settings.LOG_OUTGOING_REQUESTS_DB_SAVE:
-            from .models import OutgoingRequestsLog
-
-            trace = None
+        if config.save_logs_enabled:
+            trace = ""
 
             # skip requests not coming from the library requests
             if not record or not record.getMessage() == "Outgoing request":
                 return
 
-            # skip requests with non-allowed content
-            request_content_type = record.req.headers.get("Content-Type", "")
-            response_content_type = record.res.headers.get("Content-Type", "")
-
-            if not (
-                request_content_type in ALLOWED_CONTENT_TYPES
-                and response_content_type in ALLOWED_CONTENT_TYPES
-            ):
-                return
-
-            safe_req_headers = record.req.headers.copy()
+            scrubbed_req_headers = record.req.headers.copy()
 
-            if "Authorization" in safe_req_headers:
-                safe_req_headers["Authorization"] = "***hidden***"
+            if "Authorization" in scrubbed_req_headers:
+                scrubbed_req_headers["Authorization"] = "***hidden***"
 
             if record.exc_info:
                 trace = traceback.format_exc()
 
             parsed_url = urlparse(record.req.url)
             kwargs = {
                 "url": record.req.url,
-                "hostname": parsed_url.hostname,
+                "hostname": parsed_url.netloc,
                 "params": parsed_url.params,
                 "status_code": record.res.status_code,
                 "method": record.req.method,
-                "req_content_type": record.req.headers.get("Content-Type", ""),
-                "res_content_type": record.res.headers.get("Content-Type", ""),
                 "timestamp": record.requested_at,
                 "response_ms": int(record.res.elapsed.total_seconds() * 1000),
-                "req_headers": self.format_headers(safe_req_headers),
+                "req_headers": self.format_headers(scrubbed_req_headers),
                 "res_headers": self.format_headers(record.res.headers),
                 "trace": trace,
             }
 
-            if config.save_body or settings.LOG_OUTGOING_REQUESTS_SAVE_BODY:
-                kwargs["req_body"] = (record.req.body,)
-                kwargs["res_body"] = (record.res.json(),)
+            if config.save_body_enabled:
+                # check request
+                content_type, encoding = parse_content_type_header(record.req)
+                if check_content_type(content_type) and check_content_length(
+                    record.req, config
+                ):
+                    kwargs["req_content_type"] = content_type
+                    kwargs["req_body"] = record.req.body or b""
+                    kwargs["req_body_encoding"] = encoding or get_default_encoding(
+                        content_type
+                    )
+
+                # check response
+                content_type, encoding = parse_content_type_header(record.res)
+                if check_content_type(content_type) and check_content_length(
+                    record.res, config
+                ):
+                    kwargs["res_content_type"] = content_type
+                    kwargs["res_body"] = record.res.content or b""
+                    kwargs[
+                        "res_body_encoding"
+                    ] = record.res.encoding or get_default_encoding(content_type)
 
             OutgoingRequestsLog.objects.create(**kwargs)