From d4d1df03c9ecbaa1920ad45fbd97a04c76b22fdb Mon Sep 17 00:00:00 2001 From: Harsha Narayana Date: Wed, 7 Nov 2018 21:32:34 +0530 Subject: [PATCH 1/2] fix content length mismatch in windows and other platform The current implementation of `sanic` attempts to make use of `ujson` if it's available in the system and if not, it will default to the inbuilt `json` module provided by python. The current implementation of `ujson` does not provide a mechanism to provide a custom `seperators` parameter as part of the `dumps` method invocation and the default behavior of the module is to strip all the spaces around seperators such as `:` and `,`. This leads to an inconsistency in the response length when the response is generated using the `ujson` and in built `json` module provided by python. To maintain the consistency, this commit overrides the default behavior of the `dumps` method provided by the `json` module to add a `seperators` argument that will strip the white spaces around these character like the default behavior of `ujson` This addresses the issue referenced in #1398 Signed-off-by: Harsha Narayana --- sanic/response.py | 7 ++++++- tests/test_response.py | 18 ++++++------------ 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/sanic/response.py b/sanic/response.py index 09c92225..7b245a82 100644 --- a/sanic/response.py +++ b/sanic/response.py @@ -1,3 +1,4 @@ +from functools import partial from mimetypes import guess_type from os import path from urllib.parse import quote_plus @@ -12,7 +13,11 @@ from sanic.helpers import STATUS_CODES, has_message_body, remove_entity_headers try: from ujson import dumps as json_dumps except BaseException: - from json import dumps as json_dumps + from json import dumps + + # This is done in order to ensure that the JSON response is + # kept consistent across both ujson and inbuilt json usage. + json_dumps = partial(dumps, separators=(",", ":")) class BaseHTTPResponse: diff --git a/tests/test_response.py b/tests/test_response.py index 7b679f43..ecf86793 100644 --- a/tests/test_response.py +++ b/tests/test_response.py @@ -1,20 +1,19 @@ -import sys import asyncio import inspect import os -from aiofiles import os as async_os from mimetypes import guess_type +from random import choice +from unittest.mock import MagicMock from urllib.parse import unquote import pytest -from random import choice +from aiofiles import os as async_os from sanic.response import ( HTTPResponse, stream, StreamingHTTPResponse, file, file_stream, json ) from sanic.server import HttpProtocol from sanic.testing import HOST, PORT -from unittest.mock import MagicMock JSON_DATA = {'ok': True} @@ -38,7 +37,6 @@ async def sample_streaming_fn(response): def test_method_not_allowed(app): - @app.get('/') async def test_get(request): return response.json({'hello': 'world'}) @@ -55,12 +53,12 @@ def test_method_not_allowed(app): request, response = app.test_client.head('/') assert response.status == 405 - assert set(response.headers['Allow'].split(', ')) == set(['GET', 'POST']) + assert set(response.headers['Allow'].split(', ')) == {'GET', 'POST'} assert response.headers['Content-Length'] == '0' request, response = app.test_client.patch('/') assert response.status == 405 - assert set(response.headers['Allow'].split(', ')) == set(['GET', 'POST']) + assert set(response.headers['Allow'].split(', ')) == {'GET', 'POST'} assert response.headers['Content-Length'] == '0' @@ -74,15 +72,11 @@ def test_response_header(app): 'CONTENT-TYPE': 'application/json' }) - is_windows = sys.platform in ['win32', 'cygwin'] request, response = app.test_client.get('/') assert dict(response.headers) == { 'Connection': 'keep-alive', 'Keep-Alive': str(app.config.KEEP_ALIVE_TIMEOUT), - # response body contains an extra \r at the end if its windows - # TODO: this is the only place this difference shows up in our tests - # we should figure out a way to unify testing on both platforms - 'Content-Length': '12' if is_windows else '11', + 'Content-Length': '11', 'Content-Type': 'application/json', } From f1f1b8a630d325286791ca97f76d0593ae3db752 Mon Sep 17 00:00:00 2001 From: Harsha Narayana Date: Wed, 7 Nov 2018 22:07:28 +0530 Subject: [PATCH 2/2] add additional test cases to validate Content-Length header Signed-off-by: Harsha Narayana --- tests/test_response.py | 47 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/tests/test_response.py b/tests/test_response.py index ecf86793..d07eaa64 100644 --- a/tests/test_response.py +++ b/tests/test_response.py @@ -81,6 +81,53 @@ def test_response_header(app): } +def test_response_content_length(app): + @app.get("/response_with_space") + async def response_with_space(request): + return json({ + "message": "Data", + "details": "Some Details" + }, headers={ + 'CONTENT-TYPE': 'application/json' + }) + + @app.get("/response_without_space") + async def response_without_space(request): + return json({ + "message":"Data", + "details":"Some Details" + }, headers={ + 'CONTENT-TYPE': 'application/json' + }) + + _, response = app.test_client.get("/response_with_space") + content_length_for_response_with_space = response.headers.get("Content-Length") + + _, response = app.test_client.get("/response_without_space") + content_length_for_response_without_space = response.headers.get("Content-Length") + + assert content_length_for_response_with_space == content_length_for_response_without_space + + assert content_length_for_response_with_space == '43' + + +def test_response_content_length_with_different_data_types(app): + @app.get("/") + async def get_data_with_different_types(request): + # Indentation issues in the Response is intentional. Please do not fix + return json({ + 'bool': True, + 'none': None, + 'string':'string', + 'number': -1}, + headers={ + 'CONTENT-TYPE': 'application/json' + }) + + _, response = app.test_client.get("/") + assert response.headers.get("Content-Length") == '55' + + @pytest.fixture def json_app(app):