Repository: huge-success/sanic
Pull request #1776 (2020-01-30 03:53:41)

Bug fix for host parameter issue with lists
As explained in #1772 there is an issue when using a list as an argument for the host parameter in the Blueprint.route() decorator. I've traced the issue back to this line, and the if conditional should ensure that the name attribute isn't accessed when route is None.

codecov[bot]: # [Codecov](https://codecov.io/gh/huge-success/sanic/pull/1776?src=pr&el=h1) Report
> Merging [#1776](https://codecov.io/gh/huge-success/sanic/pull/1776?src=pr&el=desc) into [master](https://codecov.io/gh/huge-success/sanic/commit/dac05144415393742218a2c556bc8e037f7a2a52?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `n/a`.

[![Impacted file tree graph](https://codecov.io/gh/huge-success/sanic/pull/1776/graphs/tree.svg?width=650&token=NqLbJHUqoR&height=150&src=pr)](https://codecov.io/gh/huge-success/sanic/pull/1776?src=pr&el=tree)

```diff
@@           Coverage Diff           @@
##           master    #1776   +/-   ##
=======================================
  Coverage   91.95%   91.95%           
=======================================
  Files          23       23           
  Lines        2276     2276           
  Branches      426      426           
=======================================
  Hits         2093     2093           
  Misses        142      142           
  Partials       41       41
```



------

[Continue to review full report at Codecov](https://codecov.io/gh/huge-success/sanic/pull/1776?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/huge-success/sanic/pull/1776?src=pr&el=footer). Last update [dac0514...61c7299](https://codecov.io/gh/huge-success/sanic/pull/1776?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

damianj: @ahopkins I think the tests are being addressed in #1775? Unless that is not addressing Blueprint.route() tests.
damianj: @ahopkins I added some unit tests for using lists with the host parameter. There were already some for testing strings with the host parameter.
sjsadowski: @ahopkins can you resolve your change request if you think that we've met your threshhold?
damianj: Just following up to see if there is anything else I can or need to do in order to get this merged?
stale[bot]: This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If this  is incorrect, please respond with an update. Thank you for your contributions.

Tronic: > This seems like an okay change. But, it might be nice to have a test with host routing with a string and a list of strings.
> 
> Thanks for the PR.

#1775 was closed (stale) and this now has tests. Can we resolve this change request and get this merged?

Patch:
diff --git a/sanic/blueprints.py b/sanic/blueprints.py
index 45073078..42599342 100644
--- a/sanic/blueprints.py
+++ b/sanic/blueprints.py
@@ -143,7 +143,7 @@ class Blueprint:
             if _routes:
                 routes += _routes
 
-        route_names = [route.name for route in routes]
+        route_names = [route.name for route in routes if route]
         # Middleware
         for future in self.middlewares:
             if future.args or future.kwargs:


Test patch:
diff --git a/tests/test_blueprints.py b/tests/test_blueprints.py
index 6d5404d3..00bee043 100644
--- a/tests/test_blueprints.py
+++ b/tests/test_blueprints.py
@@ -252,6 +252,76 @@ def test_several_bp_with_host(app):
     assert response.text == "Hello3"
 
 
+def test_bp_with_host_list(app):
+    bp = Blueprint("test_bp_host", url_prefix="/test1", host=["example.com", "sub.example.com"])
+
+    @bp.route("/")
+    def handler1(request):
+        return text("Hello")
+
+    @bp.route("/", host=["sub1.example.com"])
+    def handler2(request):
+        return text("Hello subdomain!")
+
+    app.blueprint(bp)
+    headers = {"Host": "example.com"}
+    request, response = app.test_client.get("/test1/", headers=headers)
+    assert response.text == "Hello"
+
+    headers = {"Host": "sub.example.com"}
+    request, response = app.test_client.get("/test1/", headers=headers)
+    assert response.text == "Hello"
+
+    headers = {"Host": "sub1.example.com"}
+    request, response = app.test_client.get("/test1/", headers=headers)
+
+    assert response.text == "Hello subdomain!"
+
+
+def test_several_bp_with_host_list(app):
+    bp = Blueprint("test_text", url_prefix="/test", host=["example.com", "sub.example.com"])
+    bp2 = Blueprint("test_text2", url_prefix="/test", host=["sub1.example.com", "sub2.example.com"])
+
+    @bp.route("/")
+    def handler(request):
+        return text("Hello")
+
+    @bp2.route("/")
+    def handler1(request):
+        return text("Hello2")
+
+    @bp2.route("/other/")
+    def handler2(request):
+        return text("Hello3")
+
+    app.blueprint(bp)
+    app.blueprint(bp2)
+
+    assert bp.host == ["example.com", "sub.example.com"]
+    headers = {"Host": "example.com"}
+    request, response = app.test_client.get("/test/", headers=headers)
+    assert response.text == "Hello"
+
+    assert bp.host == ["example.com", "sub.example.com"]
+    headers = {"Host": "sub.example.com"}
+    request, response = app.test_client.get("/test/", headers=headers)
+    assert response.text == "Hello"
+
+    assert bp2.host == ["sub1.example.com", "sub2.example.com"]
+    headers = {"Host": "sub1.example.com"}
+    request, response = app.test_client.get("/test/", headers=headers)
+    assert response.text == "Hello2"
+    request, response = app.test_client.get("/test/other/", headers=headers)
+    assert response.text == "Hello3"
+
+    assert bp2.host == ["sub1.example.com", "sub2.example.com"]
+    headers = {"Host": "sub2.example.com"}
+    request, response = app.test_client.get("/test/", headers=headers)
+    assert response.text == "Hello2"
+    request, response = app.test_client.get("/test/other/", headers=headers)
+    assert response.text == "Hello3"
+
+
 def test_bp_middleware(app):
     blueprint = Blueprint("test_bp_middleware")