Dont bind IP to User if invalid

src/sentry/event_manager.py

@@ -39,6 +39,7 @@
 from sentry.utils.db import get_db_engine
 from sentry.utils.safe import safe_execute, trim, trim_dict
 from sentry.utils.strings import truncatechars
+from sentry.utils.validators import validate_ip
 
 # Valid pattern for tag key names
 TAG_KEY_RE = re.compile(r'^[a-zA-Z0-9_\.:-]+$')
@@ -303,8 +304,14 @@ def normalize(self):
             del data['sentry.interfaces.Stacktrace']
 
         if 'sentry.interfaces.Http' in data:
-            ip_address = data['sentry.interfaces.Http'].get(
-                'env', {}).get('REMOTE_ADDR')
+            try:
+                ip_address = validate_ip(
+                    data['sentry.interfaces.Http'].get(
+                        'env', {}).get('REMOTE_ADDR'),
+                    required=False,
+                )
+            except ValueError:
+                ip_address = None
             if ip_address:
                 data.setdefault('sentry.interfaces.User', {})
                 data['sentry.interfaces.User'].setdefault(

src/sentry/interfaces/user.py

@@ -12,7 +12,7 @@
 from sentry.interfaces.base import Interface, InterfaceValidationError
 from sentry.utils.safe import trim, trim_dict
 from sentry.web.helpers import render_to_string
-from ipaddr import IPAddress
+from sentry.utils.validators import validate_ip
 
 
 def validate_email(value, required=True):
@@ -28,15 +28,6 @@ def validate_email(value, required=True):
     return value
 
 
-def validate_ip(value, required=True):
-    if not required and not value:
-        return
-
-    # will raise a ValueError
-    IPAddress(value)
-    return value
-
-
 class User(Interface):
     """
     An interface which describes the authenticated User for a request.

src/sentry/utils/validators.py

@@ -0,0 +1,12 @@
+from __future__ import absolute_import
+
+from ipaddr import IPAddress
+
+
+def validate_ip(value, required=True):
+    if not required and not value:
+        return
+
+    # will raise a ValueError
+    IPAddress(value)
+    return value

tests/sentry/test_event_manager.py

@@ -69,6 +69,45 @@ def test_interface_is_relabeled(self):
         assert data['sentry.interfaces.User'] == {'id': '1'}
         assert 'user' not in data
 
+    def test_does_default_ip_address_to_user(self):
+        manager = EventManager(self.make_event(**{
+            'sentry.interfaces.Http': {
+                'url': 'http://example.com',
+                'env': {
+                    'REMOTE_ADDR': '127.0.0.1',
+                }
+            }
+        }))
+        data = manager.normalize()
+        assert data['sentry.interfaces.User']['ip_address'] == '127.0.0.1'
+
+    def test_does_default_ip_address_if_present(self):
+        manager = EventManager(self.make_event(**{
+            'sentry.interfaces.Http': {
+                'url': 'http://example.com',
+                'env': {
+                    'REMOTE_ADDR': '127.0.0.1',
+                }
+            },
+            'sentry.interfaces.User': {
+                'ip_address': '192.168.0.1',
+            },
+        }))
+        data = manager.normalize()
+        assert data['sentry.interfaces.User']['ip_address'] == '192.168.0.1'
+
+    def test_does_not_default_invalid_ip_address(self):
+        manager = EventManager(self.make_event(**{
+            'sentry.interfaces.Http': {
+                'url': 'http://example.com',
+                'env': {
+                    'REMOTE_ADDR': '127.0.0.1, 192.168.0.1',
+                }
+            }
+        }))
+        data = manager.normalize()
+        assert 'sentry.interfaces.User' not in data
+
     def test_platform_is_saved(self):
         manager = EventManager(self.make_event(platform='python'))
         event = manager.save(1)