From fc4cb5735702324b502ceecd5825a93406596a3e Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 28 Jan 2013 18:11:27 -0800 Subject: [PATCH] Fix JSON encoding of PhutilSafeHTML for browser consumption Summary: If you run this code: json_encode(array('tag' => phutil_tag('div', array()))); ...you get this result, because json_encode() does not call toString() on objects: {"tag":{}} Instead, convert such objects to their underlying strings. Javelin has support for JX.HTML and for implicit conversion (which is kind of sketchy for other reasons) but it's sort of complicated (only happens on Ajax, not behaviors) and messy (not metadata-based), so ignore it for now. We'll need to do something similar for serialization to the database. My plan there is just to throw on any objects. The only time we put HTML in the database is cache-related and those tiny number of callsites can manually handle it. Test Plan: Various ajax things now receive the correct data. Reviewers: vrana Reviewed By: vrana CC: aran Maniphest Tasks: T2432 Differential Revision: https://secure.phabricator.com/D4684 --- externals/javelinjs/src/lib/DOM.js | 11 +++++++++++ src/aphront/response/AphrontResponse.php | 16 +++++++++++++++- .../celerity/CelerityStaticResourceResponse.php | 7 +++++-- 3 files changed, 31 insertions(+), 3 deletions(-) diff --git a/externals/javelinjs/src/lib/DOM.js b/externals/javelinjs/src/lib/DOM.js index c51b43ed7b..b2bf50b65c 100644 --- a/externals/javelinjs/src/lib/DOM.js +++ b/externals/javelinjs/src/lib/DOM.js @@ -87,7 +87,18 @@ JX.$ = function(id) { JX.install('HTML', { construct : function(str) { + if (str instanceof JX.HTML) { + this._content = str._content; + return; + } + if (__DEV__) { + if ((typeof str !== 'string') && (!str || !str.match)) { + JX.$E( + 'new JX.HTML(): ' + + 'call initializes an HTML object with an empty value.'); + } + var tags = ['legend', 'thead', 'tbody', 'tfoot', 'column', 'colgroup', 'caption', 'tr', 'th', 'td', 'option']; var evil_stuff = new RegExp('^\\s*<(' + tags.join('|') + ')\\b', 'i'); diff --git a/src/aphront/response/AphrontResponse.php b/src/aphront/response/AphrontResponse.php index 291c493c59..e410262b39 100644 --- a/src/aphront/response/AphrontResponse.php +++ b/src/aphront/response/AphrontResponse.php @@ -54,7 +54,21 @@ abstract class AphrontResponse { return $this; } - protected function encodeJSONForHTTPResponse(array $object) { + public static function processValueForJSONEncoding(&$value, $key) { + if ($value instanceof PhutilSafeHTML) { + // TODO: Javelin supports implicity conversion of '__html' objects to + // JX.HTML, but only for Ajax responses, not behaviors. Just leave things + // as they are for now (where behaviors treat responses as HTML or plain + // text at their discretion). + $value = $value->getHTMLContent(); + } + } + + public static function encodeJSONForHTTPResponse(array $object) { + + array_walk_recursive( + $object, + array('AphrontResponse', 'processValueForJSONEncoding')); $response = json_encode($object); diff --git a/src/infrastructure/celerity/CelerityStaticResourceResponse.php b/src/infrastructure/celerity/CelerityStaticResourceResponse.php index e16d75f799..6891fa4fff 100644 --- a/src/infrastructure/celerity/CelerityStaticResourceResponse.php +++ b/src/infrastructure/celerity/CelerityStaticResourceResponse.php @@ -128,7 +128,8 @@ final class CelerityStaticResourceResponse { public function renderHTMLFooter() { $data = array(); if ($this->metadata) { - $json_metadata = json_encode($this->metadata); + $json_metadata = AphrontResponse::encodeJSONForHTTPResponse( + $this->metadata); $this->metadata = array(); } else { $json_metadata = '{}'; @@ -164,7 +165,9 @@ final class CelerityStaticResourceResponse { if (!$group) { continue; } - $onload[] = 'JX.initBehaviors('.json_encode($group).')'; + $group_json = AphrontResponse::encodeJSONForHTTPResponse( + $group); + $onload[] = 'JX.initBehaviors('.$group_json.')'; } }