Skip to content

Fix random disconnect after upgrade to encrypted#253

Merged
ikalchev merged 1 commit intoikalchev:devfrom
bdraco:fix_connection_drop_after_upgrade_to_encrypted
May 15, 2020
Merged

Fix random disconnect after upgrade to encrypted#253
ikalchev merged 1 commit intoikalchev:devfrom
bdraco:fix_connection_drop_after_upgrade_to_encrypted

Conversation

@bdraco
Copy link
Copy Markdown
Contributor

@bdraco bdraco commented May 15, 2020

After server restarts accessories would sometimes go
unavailable because after reconnecting and switching
to encrypted transport part of the unencrypted response
would not have been sent down the wire. After the
upgrade to encrypted happens that response gets lost
which means the server starts sending encrypted data
when the hap client/controller is still expecting
to finish reading the unencrypted data.

The net result is that the controller/client has to
try again to get another connection to the server
where it does not get unluckly and hit this race
condition.

Before the change "Cleaning connection" was
very common in the log. Its now rare.

Now that Apple has published more information,
we can improve conformance to HAP spec.

Content-Length is no longer sent on empty content
as it could lead to a loop on the client/controller side

The Connection, Date, and Server headers are not expected to be
sent according to the spec, and they are now suppressed.

After server restarts accessories would sometimes go
unavailable because after reconnecting and switching
to encrypted transport part of the unencrypted response
would not have been sent down the wire.  After the
upgrade to encrypted happens that response gets lost
which means the server starts sending encrypted data
when the hap client/controller is still expecting
to finish reading the unencrypted data.

The net result is that the controller/client has to
try again to get another connection to the server
where it does not get unluckly and hit this race
condition.

Before the change "Cleaning connection" was
very common in the log.  Its now rare.

Now that Apple has published more information,
we can improve conformance to HAP spec.

Content-Length is no longer sent on empty content
as it could lead to a loop on the client/controlller side

The Connection, Date, and Server headers are not expected to be
sent according to the spec, and they are now suppressed.
@bdraco
Copy link
Copy Markdown
Contributor Author

bdraco commented May 15, 2020

Manual Testing steps:

Turn on airplane mode, turn off wifi, wait for accessories to go unavailable.

Turn back on wifi and airplane mode off. Watch the log. Verify no double connects

@codecov-io
Copy link
Copy Markdown

Codecov Report

Merging #253 into dev will increase coverage by 0.14%.
The diff coverage is 88.88%.

@@            Coverage Diff             @@
##              dev     #253      +/-   ##
==========================================
+ Coverage   62.96%   63.10%   +0.14%     
==========================================
  Files          16       16              
  Lines        1736     1743       +7     
  Branches      186      187       +1     
==========================================
+ Hits         1093     1100       +7     
  Misses        591      591              
  Partials       52       52              
Impacted Files Coverage Δ
pyhap/hap_server.py 38.09% <88.88%> (+0.87%) ⬆️

@ikalchev ikalchev merged commit 6319e4a into ikalchev:dev May 15, 2020
@ikalchev
Copy link
Copy Markdown
Owner

Great job!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants