From e81a8ce07329e95d3d0899b1d774f21759c28e0e Mon Sep 17 00:00:00 2001 From: Harsha Narayana Date: Fri, 1 Nov 2019 23:02:49 +0530 Subject: [PATCH] fix SERVER_NAME enforcement in url_for and request.args documentation (#1708) * :bug: fix SERVER_NAME enforcement in url_for fixes #1707 * :bulb: add additional documentation for using request.args fixes #1704 * :white_check_mark: add additional test to check url_for without SERVER_NAME * :pencil: add changelog for fixes --- changelogs/1704.doc.rst | 3 +++ changelogs/1707.bugfix.rst | 4 ++++ docs/sanic/request_data.md | 18 +++++++++++++++--- sanic/request.py | 7 +++++-- tests/test_requests.py | 16 ++++++++++++++++ 5 files changed, 43 insertions(+), 5 deletions(-) create mode 100644 changelogs/1704.doc.rst create mode 100644 changelogs/1707.bugfix.rst diff --git a/changelogs/1704.doc.rst b/changelogs/1704.doc.rst new file mode 100644 index 00000000..873ce539 --- /dev/null +++ b/changelogs/1704.doc.rst @@ -0,0 +1,3 @@ +Fix documentation for `get` and `getlist` of the `request.args` + +Add additional example for showing the usage of `getlist` and fix the documentation string for `request.args` behavior diff --git a/changelogs/1707.bugfix.rst b/changelogs/1707.bugfix.rst new file mode 100644 index 00000000..a98cd6b4 --- /dev/null +++ b/changelogs/1707.bugfix.rst @@ -0,0 +1,4 @@ +Fix `url_for` behavior with missing SERVER_NAME + +If the `SERVER_NAME` was missing in the `app.config` entity, the `url_for` on the `request` and `app` were failing +due to an `AttributeError`. This fix makes the availability of `SERVER_NAME` on our `app.config` an optional behavior. diff --git a/docs/sanic/request_data.md b/docs/sanic/request_data.md index 34f87bd5..eb8f12fa 100644 --- a/docs/sanic/request_data.md +++ b/docs/sanic/request_data.md @@ -204,9 +204,8 @@ The output will be: ## Accessing values using `get` and `getlist` -The request properties which return a dictionary actually return a subclass of -`dict` called `RequestParameters`. The key difference when using this object is -the distinction between the `get` and `getlist` methods. +The `request.args` returns a subclass of `dict` called `RequestParameters`. +The key difference when using this object is the distinction between the `get` and `getlist` methods. - `get(key, default=None)` operates as normal, except that when the value of the given key is a list, *only the first item is returned*. @@ -223,6 +222,19 @@ args.get('titles') # => 'Post 1' args.getlist('titles') # => ['Post 1', 'Post 2'] ``` +```python +from sanic import Sanic +from sanic.response import json + +app = Sanic(name="example") + +@app.route("/") +def get_handler(request): + return json({ + "p1": request.args.getlist("p1") + }) +``` + ## Accessing the handler name with the request.endpoint attribute The `request.endpoint` attribute holds the handler's name. For instance, the below diff --git a/sanic/request.py b/sanic/request.py index 972f7dbf..9712aeb3 100644 --- a/sanic/request.py +++ b/sanic/request.py @@ -519,8 +519,11 @@ class Request: :rtype: str """ # Full URL SERVER_NAME can only be handled in app.url_for - if "//" in self.app.config.SERVER_NAME: - return self.app.url_for(view_name, _external=True, **kwargs) + try: + if "//" in self.app.config.SERVER_NAME: + return self.app.url_for(view_name, _external=True, **kwargs) + except AttributeError: + pass scheme = self.scheme host = self.server_name diff --git a/tests/test_requests.py b/tests/test_requests.py index 5d0062e4..2f83513c 100644 --- a/tests/test_requests.py +++ b/tests/test_requests.py @@ -2103,3 +2103,19 @@ async def test_endpoint_blueprint_asgi(): request, response = await app.asgi_client.get("/bp") assert request.endpoint == "named.my_blueprint.bp_root" + + +def test_url_for_without_server_name(app): + @app.route("/sample") + def sample(request): + return json({"url": request.url_for("url_for")}) + + @app.route("/url-for") + def url_for(request): + return text("url-for") + + request, response = app.test_client.get("/sample") + assert ( + response.json["url"] + == f"http://127.0.0.1:{app.test_client.port}/url-for" + )