From 181bfffaa126f4e63eeb24d6c5dbb459d735a32c Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 18 Dec 2013 11:59:53 -0800
Subject: [PATCH] Truncate object fields in Herald transcripts

Summary:
A few users have hit cases where Herald transcripts of large commits exceed the MySQL packet limit, because one of the fields in the transcript is an enomrous textual diff.

There's no value in saving these huge amounts of data. Transcripts are useful for understanding the action of Herald rules, but can be reconstructed later. Instead of saving all of the data, limit each field to 4KB of data.

For strings, we just truncate at 4KB. For arrays, we truncate after 4KB of values.

Test Plan: Ran unit tests. Artificially decreased limit and ran transcripts, saw them truncate properly.

Reviewers: btrahan

Reviewed By: btrahan

CC: frgtn, aran

Differential Revision: https://secure.phabricator.com/D7783
---
 src/__phutil_library_map__.php                |  2 +
 .../__tests__/HeraldTranscriptTestCase.php    | 47 +++++++++++++++++++
 .../transcript/HeraldObjectTranscript.php     | 30 ++++++++++++
 3 files changed, 79 insertions(+)
 create mode 100644 src/applications/herald/storage/__tests__/HeraldTranscriptTestCase.php

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index 5262157a91..62047c89da 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -779,6 +779,7 @@ phutil_register_library_map(array(
     'HeraldTranscriptController' => 'applications/herald/controller/HeraldTranscriptController.php',
     'HeraldTranscriptListController' => 'applications/herald/controller/HeraldTranscriptListController.php',
     'HeraldTranscriptQuery' => 'applications/herald/query/HeraldTranscriptQuery.php',
+    'HeraldTranscriptTestCase' => 'applications/herald/storage/__tests__/HeraldTranscriptTestCase.php',
     'Javelin' => 'infrastructure/javelin/Javelin.php',
     'JavelinReactorExample' => 'applications/uiexample/examples/JavelinReactorExample.php',
     'JavelinUIExample' => 'applications/uiexample/examples/JavelinUIExample.php',
@@ -3211,6 +3212,7 @@ phutil_register_library_map(array(
     'HeraldTranscriptController' => 'HeraldController',
     'HeraldTranscriptListController' => 'HeraldController',
     'HeraldTranscriptQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
+    'HeraldTranscriptTestCase' => 'PhabricatorTestCase',
     'JavelinReactorExample' => 'PhabricatorUIExample',
     'JavelinUIExample' => 'PhabricatorUIExample',
     'JavelinViewExample' => 'PhabricatorUIExample',
diff --git a/src/applications/herald/storage/__tests__/HeraldTranscriptTestCase.php b/src/applications/herald/storage/__tests__/HeraldTranscriptTestCase.php
new file mode 100644
index 0000000000..799c8b9e34
--- /dev/null
+++ b/src/applications/herald/storage/__tests__/HeraldTranscriptTestCase.php
@@ -0,0 +1,47 @@
+<?php
+
+final class HeraldTranscriptTestCase extends PhabricatorTestCase {
+
+  public function testTranscriptTruncation() {
+    $long_string = str_repeat('x', 1024 * 1024);
+    $short_string = str_repeat('x', 4096)."\n<...>";
+
+    $long_array = array(
+      'a' => $long_string,
+      'b' => $long_string,
+    );
+
+    $mixed_array = array(
+      'a' => 'abc',
+      'b' => 'def',
+      'c' => $long_string,
+    );
+
+    $fields = array(
+      'ls' => $long_string,
+      'la' => $long_array,
+      'ma' => $mixed_array,
+    );
+
+    $truncated_fields = id(new HeraldObjectTranscript())
+      ->setFields($fields)
+      ->getFields();
+
+    $this->assertEqual($short_string, $truncated_fields['ls']);
+
+    $this->assertEqual(
+      array('a', '<...>'),
+      array_keys($truncated_fields['la']));
+    $this->assertEqual(
+      $short_string.'!<...>',
+      implode('!', $truncated_fields['la']));
+
+    $this->assertEqual(
+      array('a', 'b', 'c'),
+      array_keys($truncated_fields['ma']));
+    $this->assertEqual(
+      'abc!def!'.substr($short_string, 6),
+      implode('!', $truncated_fields['ma']));
+
+  }
+}
\ No newline at end of file
diff --git a/src/applications/herald/storage/transcript/HeraldObjectTranscript.php b/src/applications/herald/storage/transcript/HeraldObjectTranscript.php
index 701bb132fd..e63dc7d6b2 100644
--- a/src/applications/herald/storage/transcript/HeraldObjectTranscript.php
+++ b/src/applications/herald/storage/transcript/HeraldObjectTranscript.php
@@ -35,6 +35,10 @@ final class HeraldObjectTranscript {
   }
 
   public function setFields(array $fields) {
+    foreach ($fields as $key => $value) {
+      $fields[$key] = self::truncateValue($value, 4096);
+    }
+
     $this->fields = $fields;
     return $this;
   }
@@ -42,4 +46,30 @@ final class HeraldObjectTranscript {
   public function getFields() {
     return $this->fields;
   }
+
+  private static function truncateValue($value, $length) {
+    if (is_string($value)) {
+      if (strlen($value) <= $length) {
+        return $value;
+      } else {
+        // NOTE: phutil_utf8_shorten() has huge runtime for giant strings.
+        return phutil_utf8ize(substr($value, 0, $length)."\n<...>");
+      }
+    } else if (is_array($value)) {
+      foreach ($value as $key => $v) {
+        if ($length <= 0) {
+          $value['<...>'] = '<...>';
+          unset($value[$key]);
+        } else {
+          $v = self::truncateValue($v, $length);
+          $length -= strlen($v);
+          $value[$key] = $v;
+        }
+      }
+      return $value;
+    } else {
+      return $value;
+    }
+  }
+
 }