From 76c10f497fc8d92904486410f2140eaab793783e Mon Sep 17 00:00:00 2001
From: Ricky Elrod <ricky@elrod.me>
Date: Fri, 11 Jan 2013 15:28:33 -0800
Subject: [PATCH] Fix error in `PhabricatorSetupIssueView`

Summary:
- Move `prettyPrintJSON()` and make it static.
- Use it from `PhabricatorSetupIssueView`
- Update other `config/` places that use it to call it from the new class.

This fixes a bug in `PhabricatorSetupIssueView` which showed up if the value
was an array and couldn't be rendered by `phutil_escape_html()`.

Test Plan:
- Rendered some config options.
- Went to /config/issue/config.unknown.phame.skins/ without error.

Reviewers: epriestley, btrahan, chad

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2255

Differential Revision: https://secure.phabricator.com/D4411
---
 src/__phutil_library_map__.php                |  1 +
 .../PhabricatorConfigController.php           | 16 ----------------
 .../PhabricatorConfigEditController.php       |  5 +++--
 .../PhabricatorConfigGroupController.php      |  3 ++-
 .../config/json/PhabricatorConfigJSON.php     | 19 +++++++++++++++++++
 .../config/view/PhabricatorSetupIssueView.php |  3 ++-
 6 files changed, 27 insertions(+), 20 deletions(-)
 create mode 100644 src/applications/config/json/PhabricatorConfigJSON.php

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index a0cab2499c..043f32abd1 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -706,6 +706,7 @@ phutil_register_library_map(array(
     'PhabricatorConfigGroupController' => 'applications/config/controller/PhabricatorConfigGroupController.php',
     'PhabricatorConfigIssueListController' => 'applications/config/controller/PhabricatorConfigIssueListController.php',
     'PhabricatorConfigIssueViewController' => 'applications/config/controller/PhabricatorConfigIssueViewController.php',
+    'PhabricatorConfigJSON' => 'applications/config/json/PhabricatorConfigJSON.php',
     'PhabricatorConfigListController' => 'applications/config/controller/PhabricatorConfigListController.php',
     'PhabricatorConfigLocalSource' => 'infrastructure/env/PhabricatorConfigLocalSource.php',
     'PhabricatorConfigManagementSetWorkflow' => 'infrastructure/env/management/PhabricatorConfigManagementSetWorkflow.php',
diff --git a/src/applications/config/controller/PhabricatorConfigController.php b/src/applications/config/controller/PhabricatorConfigController.php
index 53e1a351c5..32c0f95481 100644
--- a/src/applications/config/controller/PhabricatorConfigController.php
+++ b/src/applications/config/controller/PhabricatorConfigController.php
@@ -21,20 +21,4 @@ abstract class PhabricatorConfigController extends PhabricatorController {
     return $this->buildSideNavView(null, true)->getMenu();
   }
 
-  /**
-   * Properly format a JSON value.
-   *
-   * @param wild Any value, but should be a raw value, not a string of JSON.
-   * @return string
-   */
-  public function prettyPrintJSON($value) {
-    // Check not only that it's an array, but that it's an "unnatural" array
-    // meaning that the keys aren't 0 -> size_of_array.
-    if (is_array($value) && array_keys($value) != range(0, count($value) - 1)) {
-      return id(new PhutilJSON())->encodeFormatted($value);
-    } else {
-      return json_encode($value);
-    }
-  }
-
 }
diff --git a/src/applications/config/controller/PhabricatorConfigEditController.php b/src/applications/config/controller/PhabricatorConfigEditController.php
index 5dafbbedf1..316cee69b2 100644
--- a/src/applications/config/controller/PhabricatorConfigEditController.php
+++ b/src/applications/config/controller/PhabricatorConfigEditController.php
@@ -330,7 +330,7 @@ final class PhabricatorConfigEditController
       case 'list<string>':
         return implode("\n", nonempty($value, array()));
       default:
-        return $this->prettyPrintJSON($value);
+        return PhabricatorConfigJSON::prettyPrintJSON($value);
     }
   }
 
@@ -462,7 +462,8 @@ final class PhabricatorConfigEditController
       if (!array_key_exists($option->getKey(), $value)) {
         $value = '<em>'.pht('(empty)').'</em>';
       } else {
-        $value = $this->prettyPrintJSON($value[$option->getKey()]);
+        $value = PhabricatorConfigJSON::prettyPrintJSON(
+          $value[$option->getKey()]);
       }
 
       $table[] = '<tr>';
diff --git a/src/applications/config/controller/PhabricatorConfigGroupController.php b/src/applications/config/controller/PhabricatorConfigGroupController.php
index 1d5042b2fd..e5f1cd0889 100644
--- a/src/applications/config/controller/PhabricatorConfigGroupController.php
+++ b/src/applications/config/controller/PhabricatorConfigGroupController.php
@@ -74,7 +74,8 @@ final class PhabricatorConfigGroupController
 
       if (!$option->getHidden()) {
         $current_value = PhabricatorEnv::getEnvConfig($option->getKey());
-        $current_value = $this->prettyPrintJSON($current_value);
+        $current_value = PhabricatorConfigJSON::prettyPrintJSON(
+          $current_value);
         $current_value = phutil_render_tag(
           'div',
           array(
diff --git a/src/applications/config/json/PhabricatorConfigJSON.php b/src/applications/config/json/PhabricatorConfigJSON.php
new file mode 100644
index 0000000000..e0e3a35106
--- /dev/null
+++ b/src/applications/config/json/PhabricatorConfigJSON.php
@@ -0,0 +1,19 @@
+<?php
+
+final class PhabricatorConfigJSON {
+  /**
+   * Properly format a JSON value.
+   *
+   * @param wild Any value, but should be a raw value, not a string of JSON.
+   * @return string
+   */
+  public static function prettyPrintJSON($value) {
+    // Check not only that it's an array, but that it's an "unnatural" array
+    // meaning that the keys aren't 0 -> size_of_array.
+    if (is_array($value) && array_keys($value) != range(0, count($value) - 1)) {
+      return id(new PhutilJSON())->encodeFormatted($value);
+    } else {
+      return json_encode($value);
+    }
+  }
+}
diff --git a/src/applications/config/view/PhabricatorSetupIssueView.php b/src/applications/config/view/PhabricatorSetupIssueView.php
index 2c9c8c1d14..2d8004ea00 100644
--- a/src/applications/config/view/PhabricatorSetupIssueView.php
+++ b/src/applications/config/view/PhabricatorSetupIssueView.php
@@ -133,7 +133,8 @@ final class PhabricatorSetupIssueView extends AphrontView {
       } else if ($value === true) {
         $value = '<em>true</em>';
       } else {
-        $value = phutil_escape_html($value);
+        $value = phutil_escape_html(
+          PhabricatorConfigJSON::prettyPrintJSON($value));
       }
 
       $table[] = '<td>'.$value.'</td>';