fixbug: IndexError in request.Request.remote_addr

[PWZER]

FixBug

# from request.py:383

remote_addrs = [
    addr
    for addr in [addr.strip() for addr in forwarded_for]
    if addr
]
if self.app.config.PROXIES_COUNT == -1:
    self._remote_addr = remote_addrs[0]  # IndexError: list index out of range

[codecov]

Codecov Report

Merging #1615 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff            @@
##           master   #1615      +/-   ##
=========================================
+ Coverage   91.59%   91.6%   +<.01%     
=========================================
  Files          19      19              
  Lines        2082    2084       +2     
  Branches      390     391       +1     
=========================================
+ Hits         1907    1909       +2     
  Misses        137     137              
  Partials       38      38

Impacted Files	Coverage Δ
sanic/request.py	97.4% <100%> (+0.02%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 966b05b...3423ae8. Read the comment docs.

[ahopkins]

Thanks for the PR. Can you explain the bug this is resolving?

[andreymal]

Oh shame, I forgot this obvious test >_<

Click me

diff --git a/tests/test_requests.py b/tests/test_requests.py
index ea1946d..a01b53c 100644
--- a/tests/test_requests.py
+++ b/tests/test_requests.py
@@ -408,6 +408,11 @@ def test_remote_addr_with_two_proxies(app):
     async def handler(request):
         return text(request.remote_addr)
 
+    headers = {}
+    request, response = app.test_client.get("/", headers=headers)
+    assert request.remote_addr == ""
+    assert response.text == ""
+
     headers = {"X-Real-IP": "127.0.0.2", "X-Forwarded-For": "127.0.1.1"}
     request, response = app.test_client.get("/", headers=headers)
     assert request.remote_addr == "127.0.0.2"
@@ -448,6 +453,11 @@ async def test_remote_addr_with_two_proxies_asgi(app):
     async def handler(request):
         return text(request.remote_addr)
 
+    headers = {}
+    request, response = await app.asgi_client.get("/", headers=headers)
+    assert request.remote_addr == ""
+    assert response.text == ""
+
     headers = {"X-Real-IP": "127.0.0.2", "X-Forwarded-For": "127.0.1.1"}
     request, response = await app.asgi_client.get("/", headers=headers)
     assert request.remote_addr == "127.0.0.2"
@@ -487,6 +497,11 @@ def test_remote_addr_with_infinite_number_of_proxies(app):
     async def handler(request):
         return text(request.remote_addr)
 
+    headers = {}
+    request, response = app.test_client.get("/", headers=headers)
+    assert request.remote_addr == ""
+    assert response.text == ""
+
     headers = {"X-Real-IP": "127.0.0.2", "X-Forwarded-For": "127.0.1.1"}
     request, response = app.test_client.get("/", headers=headers)
     assert request.remote_addr == "127.0.0.2"
@@ -513,6 +528,11 @@ async def test_remote_addr_with_infinite_number_of_proxies_asgi(app):
     async def handler(request):
         return text(request.remote_addr)
 
+    headers = {}
+    request, response = await app.asgi_client.get("/", headers=headers)
+    assert request.remote_addr == ""
+    assert response.text == ""
+
     headers = {"X-Real-IP": "127.0.0.2", "X-Forwarded-For": "127.0.1.1"}
     request, response = await app.asgi_client.get("/", headers=headers)
     assert request.remote_addr == "127.0.0.2"
@@ -538,6 +558,11 @@ def test_remote_addr_without_proxy(app):
     async def handler(request):
         return text(request.remote_addr)
 
+    headers = {}
+    request, response = app.test_client.get("/", headers=headers)
+    assert request.remote_addr == ""
+    assert response.text == ""
+
     headers = {"X-Real-IP": "127.0.0.2", "X-Forwarded-For": "127.0.1.1"}
     request, response = app.test_client.get("/", headers=headers)
     assert request.remote_addr == ""

[andreymal]

@PWZER ping?

[PWZER]

@ahopkins @andreymal

This bug is a logical problem in the code, before using remote_addrs[0] did not judge whether remote_addrs is zero-length, so get IndexError.

https://github.com/huge-success/sanic/blob/84b41123f29e8f52c63c82af6f9279a6edaaf1a0/sanic/request.py#L415-L421

The following is my modification

-                if self.app.config.PROXIES_COUNT == -1:
+                if len(remote_addrs) == 0:
+                    self._remote_addr = ""
+                elif self.app.config.PROXIES_COUNT == -1:

[Tronic]

The fix seems valid. Do note that pull request #1638 also re-implements this code, avoiding the problem.

[arcana261]

@PWZER why did you close this? :'( #1638 is still work in progress as of now!