Skip to content

Commit

Permalink
Add support for HTTP/1.1 and random non-responsive devices #197
Browse files Browse the repository at this point in the history
Homekit needs the headers and content sent
in a single write call or it will randomly
get connection reset by peer which leads
to iOS stalling and devices showing up
as non-responsive.

Without HTTP/1.1 keep-alives, 50+ devices
would cause a bridge to timeout or devices
to go non-responsive because homekit would
have to make a new connection to get / put
for each device.  With HTTP/1.1 it can avoid
all of this overhead

After this change, devices in homekit on
the bridges with > 80 devices were
now immediately responsive in testing.
  • Loading branch information
bdraco committed Dec 10, 2019
1 parent 991761c commit fbabe83
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 3 deletions.
29 changes: 26 additions & 3 deletions pyhap/hap_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

import pyhap.tlv as tlv
from pyhap.util import long_to_bytes
from pyhap.const import __version__

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -137,6 +138,13 @@ def __init__(self, sock, client_addr, server, accessory_handler):
self.state = self.accessory_handler.state
self.enc_context = None
self.is_encrypted = False
self.server_version = 'pyhap/' + __version__
# HTTP/1.1 allows a keep-alive which makes
# a large accessory list usable in homekit
# If iOS has to reconnect to query each accessory
# it can be painfully slow and lead to lock up on the
# client side as well as non-responsive devices
self.protocol_version = 'HTTP/1.1'
# Redirect separate handlers to the dispatch method
self.do_GET = self.do_POST = self.do_PUT = self.dispatch

Expand Down Expand Up @@ -190,9 +198,24 @@ def _upgrade_to_encrypted(self):
def end_response(self, bytesdata, close_connection=False):
"""Combines adding a length header and actually sending the data."""
self.send_header("Content-Length", len(bytesdata))
self.end_headers()
self.wfile.write(bytesdata)
self.close_connection = 1 if close_connection else 0
# Setting this head will take care of setting
# self.close_connection to the right value
self.send_header("Connection", ("close" if close_connection else "keep-alive"))
# Important: we need to send the headers and the
# content in a single write to avoid homekit
# on the client side stalling and making
# devices appear non-responsive.
#
# The below code does what end_headers does internally
# except it combines the headers and the content
# into a single write instead of two calls to
# self.wfile.write
#
# TODO: Is there a better way that doesn't involve
# touching _headers_buffer ?
#
self.connection.sendall(b"".join(self._headers_buffer) + b"\r\n" + bytesdata)
self._headers_buffer = []

def dispatch(self):
"""Dispatch the request to the appropriate handler method."""
Expand Down
33 changes: 33 additions & 0 deletions tests/test_hap_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,3 +43,36 @@ def raises(*args):
server.finish_request(amock, client_addr)

assert len(server.connections) == 0

def test_uses_http11():
"""Test that ``HAPServerHandler`` uses HTTP/1.1."""
amock = Mock()
from pyhap.const import __version__

with patch('pyhap.hap_server.HAPServerHandler.setup'), patch('pyhap.hap_server.HAPServerHandler.handle_one_request'), patch('pyhap.hap_server.HAPServerHandler.finish'):
handler = hap_server.HAPServerHandler("mocksock", "mockclient_addr", "mockserver", amock)
assert handler.protocol_version == "HTTP/1.1"
assert handler.server_version == 'pyhap/' + __version__

def test_end_response_is_one_send():
"""Test that ``HAPServerHandler`` sends the whole response at once."""
class ConnectionMock():
sent_bytes = []

def sendall(self, bytesdata):
self.sent_bytes.append([bytesdata])
return 1

def getsent(self):
return self.sent_bytes

amock = Mock()


with patch('pyhap.hap_server.HAPServerHandler.setup'), patch('pyhap.hap_server.HAPServerHandler.handle_one_request'), patch('pyhap.hap_server.HAPServerHandler.finish'):
handler = hap_server.HAPServerHandler("mocksock", "mockclient_addr", "mockserver", amock)
handler.request_version = 'HTTP/1.1'
handler.connection = ConnectionMock()
handler.end_response(b"body")
assert handler.connection.getsent() == [[b'Content-Length: 4\r\nConnection: keep-alive\r\n\r\nbody']]
assert handler._headers_buffer == []

0 comments on commit fbabe83

Please sign in to comment.