Merge pull request mozilla-services#4292 from willkg/1424659-spaces

fixes bug 1424659 - nix consecutive spaces in signatures
jwatt · Jan 16, 2018 · 87eb5aa · 87eb5aa
2 parents 1c0dcae + 0fd04d4
commit 87eb5aa
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 23 deletions.
diff --git a/socorro/signature/generator.py b/socorro/signature/generator.py
@@ -18,8 +18,8 @@
     SignatureRunWatchDog,
     SignatureIPCChannelError,
     SignatureIPCMessageName,
-    SigTrim,
-    SigTrunc,
+    SigFixWhitespace,
+    SigTruncate,
     SignatureJitCategory,
     SignatureParentIDNotEqualsChildID,
 )
@@ -38,8 +38,8 @@
     SignatureJitCategory(),
 
     # NOTE(willkg): These should always come last and in this order
-    SigTrim(),
-    SigTrunc(),
+    SigFixWhitespace(),
+    SigTruncate(),
 ]
 
 

diff --git a/socorro/signature/rules.py b/socorro/signature/rules.py
@@ -558,19 +558,40 @@ def action(self, raw_crash, processed_crash, notes):
         return True
 
 
-class SigTrim(Rule):
-    """ensure that the signature never has any leading or trailing white
-    spaces"""
+class SigFixWhitespace(Rule):
+    """Fix whitespace in signatures
+
+    This does the following:
+
+    * trims leading and trailing whitespace
+    * converts all non-space whitespace characters to space
+    * reduce consecutive spaces to a single space
+
+    """
+
+    WHITESPACE_RE = re.compile('\s')
+    CONSECUTIVE_WHITESPACE_RE = re.compile('\s\s+')
 
     def predicate(self, raw_crash, processed_crash):
         return isinstance(processed_crash.get('signature'), basestring)
 
     def action(self, raw_crash, processed_crash, notes):
-        processed_crash['signature'] = processed_crash['signature'].strip()
+        sig = processed_crash['signature']
+
+        # Trim leading and trailing whitespace
+        sig = sig.strip()
+
+        # Convert all non-space whitespace characters into spaces
+        sig = self.WHITESPACE_RE.sub(' ', sig)
+
+        # Reduce consecutive spaces to a single space
+        sig = self.CONSECUTIVE_WHITESPACE_RE.sub(' ', sig)
+
+        processed_crash['signature'] = sig
         return True
 
 
-class SigTrunc(Rule):
+class SigTruncate(Rule):
     """Truncates signatures down to SIGNATURE_MAX_LENGTH characters"""
 
     def predicate(self, raw_crash, processed_crash):

diff --git a/socorro/unittest/signature/test_rules.py b/socorro/unittest/signature/test_rules.py
@@ -21,8 +21,8 @@
     SignatureGenerationRule,
     SignatureJitCategory,
     SignatureRunWatchDog,
-    SigTrim,
-    SigTrunc,
+    SigFixWhitespace,
+    SigTruncate,
     SignatureShutdownTimeout,
     SignatureIPCMessageName,
     SignatureParentIDNotEqualsChildID,
@@ -1277,10 +1277,10 @@ def test_action_non_ascii_abort_message(self):
         assert processed_crash['signature'] == 'Abort | unknown | hello'
 
 
-class TestSigTrim:
+class TestSigFixWhitespace:
 
     def test_predicate_no_match(self):
-        rule = SigTrim()
+        rule = SigFixWhitespace()
 
         processed_crash = {}
         predicate_result = rule.predicate({}, processed_crash)
@@ -1291,20 +1291,29 @@ def test_predicate_no_match(self):
         assert predicate_result is False
 
     def test_predicate(self):
-        rule = SigTrim()
+        rule = SigFixWhitespace()
         processed_crash = {
             'signature': 'fooo::baar'
         }
         predicate_result = rule.predicate({}, processed_crash)
         assert predicate_result is True
 
     @pytest.mark.parametrize('signature, expected', [
-        ('all   good', 'all   good'),
-        ('all   good     ', 'all   good'),
-        ('    all   good  ', 'all   good'),
+        # Leading and trailing whitespace are removed
+        ('all   good', 'all good'),
+        ('all   good     ', 'all good'),
+        ('    all   good  ', 'all good'),
+
+        # Non-space whitespace is converted to spaces
+        ('all\tgood', 'all good'),
+        ('all\n\ngood', 'all good'),
+
+        # Multiple consecutive spaces are converted to a single space
+        ('all   good', 'all good'),
+        ('all  |  good', 'all | good'),
     ])
-    def test_action_success(self, signature, expected):
-        rule = SigTrim()
+    def test_whitespace_fixing(self, signature, expected):
+        rule = SigFixWhitespace()
         processed_crash = {
             'signature': signature
         }
@@ -1313,26 +1322,26 @@ def test_action_success(self, signature, expected):
         assert processed_crash['signature'] == expected
 
 
-class TestSigTrunc:
+class TestSigTruncate:
 
     def test_predicate_no_match(self):
-        rule = SigTrunc()
+        rule = SigTruncate()
         processed_crash = {
             'signature': '0' * 100
         }
         predicate_result = rule.predicate({}, processed_crash)
         assert predicate_result is False
 
     def test_predicate(self):
-        rule = SigTrunc()
+        rule = SigTruncate()
         processed_crash = {
             'signature': '9' * 256
         }
         predicate_result = rule.predicate({}, processed_crash)
         assert predicate_result is True
 
     def test_action_success(self):
-        rule = SigTrunc()
+        rule = SigTruncate()
         processed_crash = {
             'signature': '9' * 256
         }