Skip to content

Commit

Permalink
Fix issue with local mode bodiless responses causing clients to hang
Browse files Browse the repository at this point in the history
fixes aws#525
  • Loading branch information
stealthycoin committed Sep 20, 2017
1 parent c915880 commit 7c1e32f
Show file tree
Hide file tree
Showing 6 changed files with 124 additions and 2 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,14 @@
CHANGELOG
=========

Next Release (TBD)
==================

* Fix issue with chalice local mode causing http clients to hang on responses
with no body
(`#525 <https://github.com/aws/chalice/issues/525>`__)


1.0.2
=====

Expand Down
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ check:
# No unused imports, no undefined vars,
flake8 --ignore=E731,W503 --exclude chalice/__init__.py,chalice/compat.py --max-complexity 10 chalice/
flake8 --ignore=E731,W503,F401 --max-complexity 10 chalice/compat.py
flake8 tests/unit/ tests/functional/ tests/integration/
flake8 tests/unit/ tests/functional/
##### DOC8 ######
# Correct rst formatting for documentation
#
Expand Down
1 change: 1 addition & 0 deletions chalice/local.py
Original file line number Diff line number Diff line change
Expand Up @@ -574,6 +574,7 @@ def _send_http_response_with_body(self, code, headers, body):

def _send_http_response_no_body(self, code, headers):
# type: (int, HeaderType) -> None
headers['Content-Length'] = '0'
self.send_response(code)
for k, v in headers.items():
self.send_header(k, v)
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/test_features.py
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ def test_path_params_mapped_in_api(smoke_test_app, apig_client):
}


@retry(max_attempts=10, delay=6)
@retry(max_attempts=12, delay=6)
def _get_resource_id(apig_client, rest_api_id):
# This is the resource id for the '/path/{name}'
# route. As far as I know this is the best way to get
Expand Down
112 changes: 112 additions & 0 deletions tests/integration/test_local.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
import socket
import contextlib
from threading import Thread
from threading import Event

import pytest
import requests

from chalice import app
from chalice.local import LocalDevServer
from chalice.config import Config


class ThreadedLocalServer(Thread):
def __init__(self, port):
super(ThreadedLocalServer, self).__init__()
self._app_object = None
self._config = None
self._port = port
self._server = None
self._server_ready = Event()

def configure(self, app_object, config):
self._app_object = app_object
self._config = config

def run(self):
self._server = LocalDevServer(
self._app_object, self._config, self._port)
self._server_ready.set()
self._server.serve_forever()

def make_call(self, method, path, port, timeout=0.5):
self._server_ready.wait()
return method('http://localhost:{port}{path}'.format(
path=path, port=port), timeout=timeout)

def shutdown(self):
if self._server is not None:
self._server.server.shutdown()


@pytest.fixture
def config():
return Config()


@pytest.fixture()
def unused_tcp_port():
with contextlib.closing(socket.socket()) as sock:
sock.bind(('127.0.0.1', 0))
return sock.getsockname()[1]


@pytest.fixture()
def local_server_factory(unused_tcp_port):
threaded_server = ThreadedLocalServer(unused_tcp_port)

def create_server(app_object, config):
threaded_server.configure(app_object, config)
threaded_server.start()
return threaded_server, unused_tcp_port

try:
yield create_server
finally:
threaded_server.shutdown()


@pytest.fixture
def sample_app():
demo = app.Chalice('demo-app')

@demo.route('/', methods=['GET'])
def index():
return {'hello': 'world'}

@demo.route('/test-cors', methods=['POST'], cors=True)
def test_cors():
return {'hello': 'world'}

return demo


def test_can_accept_get_request(config, sample_app, local_server_factory):
local_server, port = local_server_factory(sample_app, config)
response = local_server.make_call(requests.get, '/', port)
assert response.status_code == 200
assert response.text == '{"hello": "world"}'


def test_can_accept_options_request(config, sample_app, local_server_factory):
local_server, port = local_server_factory(sample_app, config)
response = local_server.make_call(requests.options, '/test-cors', port)
assert response.headers['Content-Length'] == '0'
assert response.headers['Access-Control-Allow-Methods'] == 'POST,OPTIONS'
assert response.text == ''


def test_can_accept_multiple_options_request(config, sample_app,
local_server_factory):
local_server, port = local_server_factory(sample_app, config)

response = local_server.make_call(requests.options, '/test-cors', port)
assert response.headers['Content-Length'] == '0'
assert response.headers['Access-Control-Allow-Methods'] == 'POST,OPTIONS'
assert response.text == ''

response = local_server.make_call(requests.options, '/test-cors', port)
assert response.headers['Content-Length'] == '0'
assert response.headers['Access-Control-Allow-Methods'] == 'POST,OPTIONS'
assert response.text == ''
1 change: 1 addition & 0 deletions tests/unit/test_local.py
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,7 @@ def test_will_respond_with_custom_cors_enabled_options(handler):
assert 'Access-Control-Expose-Headers: Header-A,Header-B' in response
assert 'Access-Control-Max-Age: 600' in response
assert 'Access-Control-Allow-Credentials: true' in response
assert 'Content-Length: 0' in response

# Ensure that the Access-Control-Allow-Methods header is sent
# and that it sends all the correct methods over.
Expand Down

0 comments on commit 7c1e32f

Please sign in to comment.