Repository: XKNX/xknx
Pull request #854 (2022-01-29 21:45:04)

Fix route back
<!--
  You are awesome! Thanks for contributing to our project!
  Please, DO NOT DELETE ANY TEXT from this template!.
-->
## Description
<!--
Please include a summary of the change and which issue is fixed.
Please also include relevant motivation and context.
List any dependencies that are required for this change.
-->
Fix route_back connections sending to invalid address

Fixes # (issue)

## Type of change
<!--
Please tick the applicable options.
NOTE: Ticking multiple options most likely indicates
that your change is to big and it is suggested to split it into several smaller PRs.
-->

- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
- [ ] This change requires a documentation update

## Checklist:

- [x] I have performed a self-review of my own code
- [ ] The documentation has been adjusted accordingly
- [x] The changes generate no new warnings
- [x] Tests have been added that prove the fix is effective or that the feature works
- [x] The changes are documented in the changelog
- [ ] The Homeassistant plugin has been adjusted in case of new config options


codecov[bot]: # [Codecov](https://codecov.io/gh/XKNX/xknx/pull/854?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX) Report
> Merging [#854](https://codecov.io/gh/XKNX/xknx/pull/854?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX) (adec748) into [main](https://codecov.io/gh/XKNX/xknx/commit/48dd002aa78c9cbcc450581dc7dc18be6ce4f0b1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX) (48dd002) will **increase** coverage by `0.01%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/XKNX/xknx/pull/854/graphs/tree.svg?width=650&height=150&src=pr&token=irWbIygS84&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX)](https://codecov.io/gh/XKNX/xknx/pull/854?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX)

```diff
@@            Coverage Diff             @@
##             main     #854      +/-   ##
==========================================
+ Coverage   95.68%   95.70%   +0.01%     
==========================================
  Files         112      112              
  Lines        6904     6907       +3     
==========================================
+ Hits         6606     6610       +4     
+ Misses        298      297       -1     
```


| [Impacted Files](https://codecov.io/gh/XKNX/xknx/pull/854?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX) | Coverage Δ | |
|---|---|---|
| [xknx/io/tunnel.py](https://codecov.io/gh/XKNX/xknx/pull/854/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX#diff-eGtueC9pby90dW5uZWwucHk=) | `73.66% <ø> (+0.44%)` | :arrow_up: |
| [xknx/knxip/hpai.py](https://codecov.io/gh/XKNX/xknx/pull/854/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX#diff-eGtueC9rbnhpcC9ocGFpLnB5) | `100.00% <100.00%> (ø)` | |


Patch:
diff --git a/changelog.md b/changelog.md
index 9b3c19ca..fd3246d3 100644
--- a/changelog.md
+++ b/changelog.md
@@ -1,5 +1,11 @@
 # Changelog
 
+## Unreleased changes
+
+### Connection
+
+- Tunneling: Fix route_back connections sending to invalid address
+
 ## 0.19.0 Tunneling connection protocol 2022-01-18
 
 ### Devices
diff --git a/xknx/io/tunnel.py b/xknx/io/tunnel.py
index 6f8301ba..c623e043 100644
--- a/xknx/io/tunnel.py
+++ b/xknx/io/tunnel.py
@@ -184,8 +184,12 @@ class _Tunnel(Interface):
             self.communication_channel = connect.communication_channel
             # assign data_endpoint received from server
             self._data_endpoint_addr = (
-                connect.data_endpoint.ip_addr,
-                connect.data_endpoint.port,
+                (
+                    connect.data_endpoint.ip_addr,
+                    connect.data_endpoint.port,
+                )
+                if not connect.data_endpoint.route_back
+                else None
             )
             # Use the individual address provided by the tunnelling server
             self._src_address = IndividualAddress(connect.identifier)
diff --git a/xknx/knxip/hpai.py b/xknx/knxip/hpai.py
index 2a0a05ab..1a2be6d6 100644
--- a/xknx/knxip/hpai.py
+++ b/xknx/knxip/hpai.py
@@ -28,6 +28,11 @@ class HPAI:
         self.port = port
         self.protocol = protocol
 
+    @property
+    def route_back(self) -> bool:
+        """Return True if HPAI is a route back address information."""
+        return self.ip_addr == "0.0.0.0"
+
     def from_knx(self, raw: bytes) -> int:
         """Parse/deserialize from KNX/IP raw data."""
         if len(raw) < HPAI.LENGTH:


Test patch:
diff --git a/test/io_tests/tunnel_test.py b/test/io_tests/tunnel_test.py
index 1f273de7..188a6ea2 100644
--- a/test/io_tests/tunnel_test.py
+++ b/test/io_tests/tunnel_test.py
@@ -114,11 +114,23 @@ class TestUDPTunnel:
         assert self.tunnel.transport.send.call_count == 2
         await task
 
-    async def test_tunnel_connect_send_disconnect(self, time_travel):
+    @pytest.mark.parametrize(
+        "route_back,data_endpoint_addr,local_endpoint",
+        [
+            (False, ("192.168.1.2", 56789), HPAI("192.168.1.1", 12345)),
+            (True, None, HPAI()),
+        ],
+    )
+    async def test_tunnel_connect_send_disconnect(
+        self, time_travel, route_back, data_endpoint_addr, local_endpoint
+    ):
         """Test initiating a tunnelling connection."""
         local_addr = ("192.168.1.1", 12345)
-        gateway_control_addr = ("192.168.1.2", 3671)
-        gateway_data_addr = ("192.168.1.2", 56789)
+        remote_addr = ("192.168.1.2", 3671)
+        self.tunnel.route_back = route_back
+        gateway_data_endpoint = (
+            HPAI(*data_endpoint_addr) if data_endpoint_addr else HPAI()
+        )
         self.tunnel.transport.connect = AsyncMock()
         self.tunnel.transport.getsockname = Mock(return_value=local_addr)
         self.tunnel.transport.send = Mock()
@@ -127,8 +139,8 @@ class TestUDPTunnel:
         # Connect
         connect_request = ConnectRequest(
             self.xknx,
-            control_endpoint=HPAI(*local_addr),
-            data_endpoint=HPAI(*local_addr),
+            control_endpoint=local_endpoint,
+            data_endpoint=local_endpoint,
         )
         connect_frame = KNXIPFrame.init_from_body(connect_request)
 
@@ -141,15 +153,13 @@ class TestUDPTunnel:
             ConnectResponse(
                 self.xknx,
                 communication_channel=23,
-                data_endpoint=HPAI(*gateway_data_addr),
+                data_endpoint=gateway_data_endpoint,
                 identifier=7,
             )
         )
-        self.tunnel.transport.handle_knxipframe(
-            connect_response_frame, gateway_control_addr
-        )
+        self.tunnel.transport.handle_knxipframe(connect_response_frame, remote_addr)
         await connection_task
-        assert self.tunnel._data_endpoint_addr == gateway_data_addr
+        assert self.tunnel._data_endpoint_addr == data_endpoint_addr
         assert self.tunnel._src_address == IndividualAddress(7)
 
         # Send - use data endpoint
@@ -171,14 +181,14 @@ class TestUDPTunnel:
         asyncio.create_task(self.tunnel.send_telegram(test_telegram))
         await time_travel(0)
         self.tunnel.transport.send.assert_called_once_with(
-            test_telegram_frame, addr=gateway_data_addr
+            test_telegram_frame, addr=data_endpoint_addr
         )
         # skip ack and confirmation
 
         # Disconnect
         self.tunnel.transport.send.reset_mock()
         disconnect_request = DisconnectRequest(
-            self.xknx, communication_channel_id=23, control_endpoint=HPAI(*local_addr)
+            self.xknx, communication_channel_id=23, control_endpoint=local_endpoint
         )
         disconnect_frame = KNXIPFrame.init_from_body(disconnect_request)
 
@@ -192,9 +202,7 @@ class TestUDPTunnel:
                 communication_channel_id=23,
             )
         )
-        self.tunnel.transport.handle_knxipframe(
-            disconnect_response_frame, gateway_control_addr
-        )
+        self.tunnel.transport.handle_knxipframe(disconnect_response_frame, remote_addr)
         await disconnection_task
         assert self.tunnel._data_endpoint_addr is None
         self.tunnel.transport.stop.assert_called_once()
diff --git a/test/knxip_tests/hpai_test.py b/test/knxip_tests/hpai_test.py
index 8ad57cb7..5c5f9106 100644
--- a/test/knxip_tests/hpai_test.py
+++ b/test/knxip_tests/hpai_test.py
@@ -16,9 +16,11 @@ class TestKNXIPHPAI:
         assert hpai.ip_addr == "192.168.42.1"
         assert hpai.port == 33941
         assert hpai.protocol == HostProtocol.IPV4_UDP
+        assert not hpai.route_back
 
         hpai2 = HPAI(ip_addr="192.168.42.1", port=33941)
         assert hpai2.to_knx() == list(raw)
+        assert not hpai2.route_back
 
     def test_hpai_tcp(self):
         """Test parsing and streaming HPAI KNX/IP fragment."""
@@ -29,6 +31,7 @@ class TestKNXIPHPAI:
         assert hpai.ip_addr == "0.0.0.0"
         assert hpai.port == 0
         assert hpai.protocol == HostProtocol.IPV4_TCP
+        assert hpai.route_back
 
         hpai2 = HPAI(ip_addr="0.0.0.0", port=0, protocol=HostProtocol.IPV4_TCP)
         assert hpai2.to_knx() == list(raw)
@@ -56,3 +59,12 @@ class TestKNXIPHPAI:
         hpai = HPAI(ip_addr=127001)
         with pytest.raises(ConversionError):
             hpai.to_knx()
+
+    def test_route_back(self):
+        """Test route_back property."""
+        hpai = HPAI()
+        assert hpai.ip_addr == "0.0.0.0"
+        assert hpai.port == 0
+        assert hpai.route_back
+        hpai.ip_addr = "10.1.2.3"
+        assert not hpai.route_back