From 980d6cb70b944986f3382ae846f7c3fee36fd1d1 Mon Sep 17 00:00:00 2001
From: Austin McKinley <bearontheroof@gmail.com>
Date: Thu, 13 Apr 2017 13:48:30 -0700
Subject: [PATCH] Add validation for config settings of type regex

Summary: Also fixes insufficiently-escaped regex examples

Test Plan: Made several changes to http://local.phacility.com/config/edit/syntax.filemap/ and observed validation failures on malformed regexes, and success on well-formed regexes.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12532

Differential Revision: https://secure.phabricator.com/D17684
---
 src/__phutil_library_map__.php                 |  2 ++
 .../PhabricatorConfigRegexOptionType.php       | 18 ++++++++++++++++++
 ...bricatorSyntaxHighlightingConfigOptions.php | 10 ++++++----
 3 files changed, 26 insertions(+), 4 deletions(-)
 create mode 100644 src/applications/config/custom/PhabricatorConfigRegexOptionType.php

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index f403a4ed7e..d9fca984f1 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -2373,6 +2373,7 @@ phutil_register_library_map(array(
     'PhabricatorConfigPageView' => 'applications/config/view/PhabricatorConfigPageView.php',
     'PhabricatorConfigProxySource' => 'infrastructure/env/PhabricatorConfigProxySource.php',
     'PhabricatorConfigPurgeCacheController' => 'applications/config/controller/PhabricatorConfigPurgeCacheController.php',
+    'PhabricatorConfigRegexOptionType' => 'applications/config/custom/PhabricatorConfigRegexOptionType.php',
     'PhabricatorConfigRequestExceptionHandlerModule' => 'applications/config/module/PhabricatorConfigRequestExceptionHandlerModule.php',
     'PhabricatorConfigResponse' => 'applications/config/response/PhabricatorConfigResponse.php',
     'PhabricatorConfigSchemaQuery' => 'applications/config/schema/PhabricatorConfigSchemaQuery.php',
@@ -7473,6 +7474,7 @@ phutil_register_library_map(array(
     'PhabricatorConfigPageView' => 'AphrontTagView',
     'PhabricatorConfigProxySource' => 'PhabricatorConfigSource',
     'PhabricatorConfigPurgeCacheController' => 'PhabricatorConfigController',
+    'PhabricatorConfigRegexOptionType' => 'PhabricatorConfigJSONOptionType',
     'PhabricatorConfigRequestExceptionHandlerModule' => 'PhabricatorConfigModule',
     'PhabricatorConfigResponse' => 'AphrontStandaloneHTMLResponse',
     'PhabricatorConfigSchemaQuery' => 'Phobject',
diff --git a/src/applications/config/custom/PhabricatorConfigRegexOptionType.php b/src/applications/config/custom/PhabricatorConfigRegexOptionType.php
new file mode 100644
index 0000000000..4f39543160
--- /dev/null
+++ b/src/applications/config/custom/PhabricatorConfigRegexOptionType.php
@@ -0,0 +1,18 @@
+<?php
+
+class PhabricatorConfigRegexOptionType
+  extends PhabricatorConfigJSONOptionType {
+
+  public function validateOption(PhabricatorConfigOption $option, $value) {
+    foreach ($value as $pattern => $spec) {
+      $ok = preg_match($pattern, '');
+      if ($ok === false) {
+        throw new Exception(
+          pht(
+            'The following regex is malformed and cannot be used: %s',
+            $pattern));
+      }
+    }
+  }
+
+}
diff --git a/src/applications/config/option/PhabricatorSyntaxHighlightingConfigOptions.php b/src/applications/config/option/PhabricatorSyntaxHighlightingConfigOptions.php
index c4e62694f8..27596f7f07 100644
--- a/src/applications/config/option/PhabricatorSyntaxHighlightingConfigOptions.php
+++ b/src/applications/config/option/PhabricatorSyntaxHighlightingConfigOptions.php
@@ -120,7 +120,7 @@ final class PhabricatorSyntaxHighlightingConfigOptions
             'this is where that list is defined.')),
       $this->newOption(
         'syntax.filemap',
-        'wild',
+        'custom:PhabricatorConfigRegexOptionType',
         array(
           '@\.arcconfig$@' => 'json',
           '@\.arclint$@' => 'json',
@@ -138,12 +138,14 @@ final class PhabricatorSyntaxHighlightingConfigOptions
             'be tested against the filename. They should map to either an '.
             'explicit language as a string value, or a numeric index into '.
             'the captured groups as an integer.'))
-      ->addExample('{"@\\.xyz$@": "php"}', pht('Highlight %s as PHP.', '*.xyz'))
       ->addExample(
-        '{"@/httpd\\.conf@": "apacheconf"}',
+        '{"@\\\.xyz$@": "php"}',
+        pht('Highlight %s as PHP.', '*.xyz'))
+      ->addExample(
+        '{"@/httpd\\\.conf@": "apacheconf"}',
         pht('Highlight httpd.conf as "apacheconf".'))
       ->addExample(
-        '{"@\\.([^.]+)\\.bak$@": 1}',
+        '{"@\\\.([^.]+)\\\.bak$@": 1}',
         pht(
           "Treat all '*.x.bak' file as '.x'. NOTE: We map to capturing group ".
           "1 by specifying the mapping as '1'")),