Skip to content

Fix edge cases with unknown IP address #111

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.

Sign up for GitHub

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
yuekui wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Fix edge cases with unknown IP address #111

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6eb0073
Fix edge cases with unknown IP addres
yuekui Jul 25, 2024
aeade52
Fix allowlist_externals error
yuekui Jul 25, 2024
b255ab7
Fix typo
yuekui Jul 25, 2024
994c8ee
Refine IP address selecting
yuekui Jul 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 15 additions & 8 deletions rest_framework_tracking/base_mixins.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,27 +111,34 @@ def _get_path(self, request):

def _get_ip_address(self, request):
"""Get the remote ip address the request was generated from."""
ipaddr = request.META.get("HTTP_X_FORWARDED_FOR", None)
if ipaddr:
ipaddr = ipaddr.split(",")[0]
else:
ipaddr = request.META.get("REMOTE_ADDR", "").split(",")[0]
raw_possibles = request.META.get("HTTP_X_FORWARDED_FOR", "").split(",")
raw_possibles += request.META.get("REMOTE_ADDR", "").split(",")
Comment on lines +114 to +115
Copy link
Contributor

@sourcery-ai sourcery-ai bot Jul 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (performance): Combining lists directly can be inefficient.

Using the += operator to combine lists can be less efficient than using extend(), especially for large lists. Consider using raw_possibles.extend(request.META.get("REMOTE_ADDR", "").split(",")) for better performance.

Suggested change
raw_possibles = request.META.get("HTTP_X_FORWARDED_FOR", "").split(",")
raw_possibles += request.META.get("REMOTE_ADDR", "").split(",")
raw_possibles = request.META.get("HTTP_X_FORWARDED_FOR", "").split(",")
raw_possibles.extend(request.META.get("REMOTE_ADDR", "").split(","))


# Account for IPv4 and IPv6 addresses, each possibly with port appended. Possibilities are:
# <ipv4 address>
# <ipv6 address>
# <ipv4 address>:port
# [<ipv6 address>]:port
# Note that ipv6 addresses are colon separated hex numbers
possibles = (ipaddr.lstrip("[").split("]")[0], ipaddr.split(":")[0])

raw_possibles = [addr.strip() for addr in raw_possibles]
possibles = []
for possible_addr in raw_possibles:
if '[' in possible_addr:
# IPv6 with port
possibles.append(possible_addr.rsplit(":", 1)[0].strip("[]"))
elif possible_addr.count(":") == 1:
# IPv4 with port
possibles.append(possible_addr.split(":")[0])
else:
# IPv4/IPv6 without port and others
possibles.append(possible_addr)
for addr in possibles:
try:
return str(ipaddress.ip_address(addr))
except ValueError:
pass

return ipaddr
return raw_possibles[0]
Copy link
Contributor

@sourcery-ai sourcery-ai bot Jul 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (bug_risk): Returning the first element of raw_possibles may not be reliable.

If raw_possibles is empty, this line will raise an IndexError. Consider adding a check to ensure raw_possibles is not empty before attempting to access its first element.


def _get_view_name(self, request):
"""Get view name."""
Expand Down
8 changes: 8 additions & 0 deletions tests/test_mixins.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,14 @@ def test_log_ip_xforwarded_list(self):
log = APIRequestLog.objects.first()
self.assertEqual(log.remote_addr, "127.0.0.8")

def test_log_ip_xforwarded_list_with_unknown_ip(self):
request = APIRequestFactory().get("/logging")
request.META["HTTP_X_FORWARDED_FOR"] = "unknown, 128.1.1.9"

MockLoggingView.as_view()(request).render()
log = APIRequestLog.objects.first()
self.assertEqual(log.remote_addr, "128.1.1.9")

def test_log_host(self):
self.client.get("/logging")
log = APIRequestLog.objects.first()
Expand Down
2 changes: 1 addition & 1 deletion tox.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ tox_pyenv_fallback = True
commands = python -V
pip install --upgrade pip pipenv
pipenv install --skip-lock
./runtests.py --fast
python runtests.py --fast
passenv =
DATABASE_URL
PYTHON_VERSION
Expand Down