search

jazzband / django-debug-toolbar

A configurable set of panels that display various debug information about the current request/response.
https://django-debug-toolbar.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
8.07k stars 1.05k forks source link

I can no longer use iptools.IpRangeList for INTERNAL_IPS #1928

Closed quinox closed 4 months ago

quinox commented 4 months ago

Hello!

Long time happy user of DDT here.

With https://github.com/jazzband/django-debug-toolbar/commit/782bdd9b0f24d3d71f6d68a20b12d5c51c6d370e I can no longer use iptools.IpRangeList with INTERNAL_IPS, it blows up when I want to whitelist a larger IP range such as IPv6. With smaller ranges such as 10.0.0.0/8 it won't blow up but it will delay every request significantly while DDT waits for IpRangeList to write out all the IP addresses.

INTERNAL_IPS = iptools.IpRangeList('127.0.0.1', '10/8', '::1', '1a:1a:1a:1a::/64')
Traceback (most recent call last):
  File "/mnt/drive/skeleton/virtual/lib/python3.10/site-packages/django/core/handlers/exception.py", line 55, in inner
    response = get_response(request)
  File "/mnt/drive/skeleton/virtual/lib/python3.10/site-packages/debug_toolbar/middleware.py", line 64, in __call__
    if not show_toolbar(request) or DebugToolbar.is_toolbar_request(request):
  File "/mnt/drive/skeleton/virtual/lib/python3.10/site-packages/debug_toolbar/middleware.py", line 23, in show_toolbar
    internal_ips = list(settings.INTERNAL_IPS)
OverflowError: cannot fit 'int' into an index-sized integer

For now I simplified my configuration to work around it. My requirements have changed a bit over the years and I can live with this simplified setup, but I just wanted to let you know in case this fallout was undesirable.

tim-schilling commented 4 months ago

Thanks for letting us know! Would you be interested in creating a PR to resolve this?

quinox commented 4 months ago

I'm happy to.

The commit doesn't state why a list was preferable. Looking around I assume the cast in https://github.com/jazzband/django-debug-toolbar/commit/782bdd9b0f24d3d71f6d68a20b12d5c51c6d370e was done because in https://github.com/jazzband/django-debug-toolbar/commit/7d77a34dcd00bcbc1561541877be132bef1789a5 the .copy()+append() was added, which didn't work on certain not-a-list objects.

For the biggest compatibility and efficiency I'd say the copy()+append() should not be done either. The code should go back to request.META.get("REMOTE_ADDR") in settings.INTERNAL_IPS. An additional clause can be added to deal with the Docker IP check separately.

I'll make a PR later today.

tim-schilling commented 4 months ago

The reason was that the suggested docker solution was to add the docker internal host IP address to INTERNAL_IPS which is defined as a list. @matthiask does the docker host need to be added to INTERNAL_IPS? If yes, @quinox you may be headed down the wrong direction.

quinox commented 4 months ago

The Docker IP is added to a local copy of the Django setting. This variable gets destroyed at the end of the function.

I made the PR: #1929. I'm happy to amend it in any way should you so desire.

matthiask commented 4 months ago

I have commented on the pull request. The change makes sense to me. INTERNAL_IPS is used by Django for a limited list of things: https://docs.djangoproject.com/en/5.0/ref/settings/#internal-ips

The toolbar won't have a problem when e.g. the debug() context processor variables aren't in the context.