From 2eee7d8888b3e444089611da975b93893d068683 Mon Sep 17 00:00:00 2001
From: Paulus Schoutsen <balloob@gmail.com>
Date: Mon, 10 Dec 2018 10:07:44 +0100
Subject: [PATCH] Drop OwnTracks bad packets

---
 .../components/owntracks/__init__.py          | 22 ++++++++++------
 tests/components/owntracks/test_init.py       | 25 ++++++++++++++++---
 2 files changed, 37 insertions(+), 10 deletions(-)

diff --git a/homeassistant/components/owntracks/__init__.py b/homeassistant/components/owntracks/__init__.py
index 0bb7a2390b7b7f..7dc88be976449a 100644
--- a/homeassistant/components/owntracks/__init__.py
+++ b/homeassistant/components/owntracks/__init__.py
@@ -118,9 +118,18 @@ async def async_handle_mqtt_message(topic, payload, qos):
 
 
 async def handle_webhook(hass, webhook_id, request):
-    """Handle webhook callback."""
+    """Handle webhook callback.
+
+    iOS sets the "topic" as part of the payload.
+    Android does not set a topic but adds headers to the request.
+    """
     context = hass.data[DOMAIN]['context']
-    message = await request.json()
+
+    try:
+        message = await request.json()
+    except ValueError:
+        _LOGGER.warning('Received invalid JSON from OwnTracks')
+        return json_response([])
 
     # Android doesn't populate topic
     if 'topic' not in message:
@@ -129,11 +138,10 @@ async def handle_webhook(hass, webhook_id, request):
         device = headers.get('X-Limit-D', user)
 
         if user is None:
-            _LOGGER.warning('Set a username in Connection -> Identification')
-            return json_response(
-                {'error': 'You need to supply username.'},
-                status=400
-            )
+            _LOGGER.warning('No topic or user found in message. If on Android,'
+                            ' set a username in Connection -> Identification')
+            # Keep it as a 200 response so the incorrect packet is discarded
+            return json_response([])
 
         topic_base = re.sub('/#$', '', context.mqtt_topic)
         message['topic'] = '{}/{}/{}'.format(topic_base, user, device)
diff --git a/tests/components/owntracks/test_init.py b/tests/components/owntracks/test_init.py
index ba362da905af2c..3d2d8d03e7c156 100644
--- a/tests/components/owntracks/test_init.py
+++ b/tests/components/owntracks/test_init.py
@@ -110,7 +110,7 @@ def test_handle_value_error(mock_client):
 
 
 @asyncio.coroutine
-def test_returns_error_missing_username(mock_client):
+def test_returns_error_missing_username(mock_client, caplog):
     """Test that an error is returned when username is missing."""
     resp = yield from mock_client.post(
         '/api/webhook/owntracks_test',
@@ -120,10 +120,29 @@ def test_returns_error_missing_username(mock_client):
         }
     )
 
-    assert resp.status == 400
+    # Needs to be 200 or OwnTracks keeps retrying bad packet.
+    assert resp.status == 200
+    json = yield from resp.json()
+    assert json == []
+    assert 'No topic or user found' in caplog.text
+
 
+@asyncio.coroutine
+def test_returns_error_incorrect_json(mock_client, caplog):
+    """Test that an error is returned when username is missing."""
+    resp = yield from mock_client.post(
+        '/api/webhook/owntracks_test',
+        data='not json',
+        headers={
+            'X-Limit-d': 'Pixel',
+        }
+    )
+
+    # Needs to be 200 or OwnTracks keeps retrying bad packet.
+    assert resp.status == 200
     json = yield from resp.json()
-    assert json == {'error': 'You need to supply username.'}
+    assert json == []
+    assert 'invalid JSON' in caplog.text
 
 
 @asyncio.coroutine