From 711871f6bc743a7e91c953695a429c37b6692ca0 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Mon, 11 Feb 2019 06:48:13 -0800
Subject: [PATCH] Allow typeaheads to pass nonscalar data to datasources

Summary:
Ref T13250. Currently, datasources have a `setParameters(...)` method. This method accepts a dictionary and adds the key/value pairs to the raw HTTP request to the datasource endpoint.

Since D20049, this no longer works. Since D20116, it fatals explicitly.

In general, the datasource endpoint accepts other values (like `query`, `offset`, and `limit`), and even before these changes, using secret reserved keys in `setParameters(...)` would silently cause program misbehavior.

To deal with this, pass parameters as a JSON string named "parameters". This fixes the HTTP query issue (the more pressing issue affecting users today) and prevents the "shadowing reserved keys" issue (a theoretical issue which might affect users some day).

(I may revisit the `phutil_build_http_querystring()` behavior and possibly let it make this work again, but I think avoiding the duplicate key issue makes this change desirable even if the querystring behavior changes.)

Test Plan:
  - Used "Land Revision", selected branches.
  - Configured a custom Maniphest "users" field, used the search typeahead, selected users.
  - Manually browsed to `/typeahead/class/PhabricatorPeopleDatasource/?query=hi&parameters=xyz` to see the JSON decode exception.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13250

Differential Revision: https://secure.phabricator.com/D20134
---
 ...orTypeaheadModularDatasourceController.php | 21 ++++++++++++++++++-
 .../PhabricatorTypeaheadDatasource.php        | 16 ++++++++++++--
 2 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/src/applications/typeahead/controller/PhabricatorTypeaheadModularDatasourceController.php b/src/applications/typeahead/controller/PhabricatorTypeaheadModularDatasourceController.php
index 9e905f8cce..7c2205df46 100644
--- a/src/applications/typeahead/controller/PhabricatorTypeaheadModularDatasourceController.php
+++ b/src/applications/typeahead/controller/PhabricatorTypeaheadModularDatasourceController.php
@@ -35,7 +35,26 @@ final class PhabricatorTypeaheadModularDatasourceController
 
     if (isset($sources[$class])) {
       $source = $sources[$class];
-      $source->setParameters($request->getRequestData());
+
+      $parameters = array();
+
+      $raw_parameters = $request->getStr('parameters');
+      if (strlen($raw_parameters)) {
+        try {
+          $parameters = phutil_json_decode($raw_parameters);
+        } catch (PhutilJSONParserException $ex) {
+          return $this->newDialog()
+            ->setTitle(pht('Invalid Parameters'))
+            ->appendParagraph(
+              pht(
+                'The HTTP parameter named "parameters" for this request is '.
+                'not a valid JSON parameter. JSON is required. Exception: %s',
+                $ex->getMessage()))
+            ->addCancelButton('/');
+        }
+      }
+
+      $source->setParameters($parameters);
       $source->setViewer($viewer);
 
       // NOTE: Wrapping the source in a Composite datasource ensures we perform
diff --git a/src/applications/typeahead/datasource/PhabricatorTypeaheadDatasource.php b/src/applications/typeahead/datasource/PhabricatorTypeaheadDatasource.php
index 2e369a3f67..196ad1b98b 100644
--- a/src/applications/typeahead/datasource/PhabricatorTypeaheadDatasource.php
+++ b/src/applications/typeahead/datasource/PhabricatorTypeaheadDatasource.php
@@ -100,7 +100,7 @@ abstract class PhabricatorTypeaheadDatasource extends Phobject {
 
   public function getDatasourceURI() {
     $uri = new PhutilURI('/typeahead/class/'.get_class($this).'/');
-    $uri->setQueryParams($this->parameters);
+    $uri->setQueryParams($this->newURIParameters());
     return (string)$uri;
   }
 
@@ -110,10 +110,22 @@ abstract class PhabricatorTypeaheadDatasource extends Phobject {
     }
 
     $uri = new PhutilURI('/typeahead/browse/'.get_class($this).'/');
-    $uri->setQueryParams($this->parameters);
+    $uri->setQueryParams($this->newURIParameters());
     return (string)$uri;
   }
 
+  private function newURIParameters() {
+    if (!$this->parameters) {
+      return array();
+    }
+
+    $map = array(
+      'parameters' => phutil_json_encode($this->parameters),
+    );
+
+    return $map;
+  }
+
   abstract public function getPlaceholderText();
 
   public function getBrowseTitle() {