Page MenuHomePhabricator
Files F15398457

D9634.id23120.diff
No OneTemporary
Actions

D9634.id23120.diff
View Options

diff --git a/externals/jsonlint/src/Seld/JsonLint/JsonParser.php b/externals/jsonlint/src/Seld/JsonLint/JsonParser.php
--- a/externals/jsonlint/src/Seld/JsonLint/JsonParser.php
+++ b/externals/jsonlint/src/Seld/JsonLint/JsonParser.php
@@ -380,11 +380,7 @@
$errStr .= "Duplicate key: ".$tokens[$len][0];
throw new JsonLintParsingException($errStr);
} elseif (($this->flags & self::ALLOW_DUPLICATE_KEYS) && array_key_exists($key, $tokens[$len-2])) {
- $duplicateCount = 1;
- do {
- $duplicateKey = $key . '.' . $duplicateCount++;
- } while (array_key_exists($duplicateKey, $tokens[$len-2]));
- $key = $duplicateKey;
+ // Forget about it...
}
$yyval->token[$key] = $tokens[$len][1];
break;
diff --git a/src/parser/PhutilJSONParser.php b/src/parser/PhutilJSONParser.php
--- a/src/parser/PhutilJSONParser.php
+++ b/src/parser/PhutilJSONParser.php
@@ -8,6 +8,12 @@
*/
final class PhutilJSONParser {
+ private $allowDuplicateKeys = false;
+
+ public function setAllowDuplicateKeys($allow_duplicate_keys) {
+ $this->allowDuplicateKeys = $allow_duplicate_keys;
+ }
+
public function parse($json) {
$jsonlint_root = phutil_get_library_root('phutil').'/../externals/jsonlint';
require_once($jsonlint_root.'/src/Seld/JsonLint/JsonParser.php');
@@ -17,9 +23,17 @@
$parser = new JsonLintJsonParser();
try {
- $output = $parser->parse($json);
+ $output = $parser->parse($json, $this->getFlags());
} catch (JsonLintParsingException $ex) {
- throw new PhutilJSONParserException($ex->getMessage());
+ $details = $ex->getDetails();
+ $message = preg_replace("/^Parse error .*\\^\n/s", '', $ex->getMessage());
+
+ throw new PhutilJSONParserException(
+ $message,
+ idx(idx($details, 'loc', array()), 'last_line'),
+ idx(idx($details, 'loc', array()), 'last_column'),
+ idx($details, 'token'),
+ idx($details, 'expected'));
}
if (!is_array($output)) {
@@ -32,4 +46,16 @@
return $output;
}
+ private function getFlags() {
+ $flags = 0;
+
+ if ($this->allowDuplicateKeys) {
+ $flags |= JsonLintJsonParser::ALLOW_DUPLICATE_KEYS;
+ } else {
+ $flags |= JsonLintJsonParser::DETECT_KEY_CONFLICTS;
+ }
+
+ return $flags;
+ }
+
}
diff --git a/src/parser/__tests__/PhutilJSONParserTestCase.php b/src/parser/__tests__/PhutilJSONParserTestCase.php
--- a/src/parser/__tests__/PhutilJSONParserTestCase.php
+++ b/src/parser/__tests__/PhutilJSONParserTestCase.php
@@ -30,19 +30,77 @@
$parser = new PhutilJSONParser();
$tests = array(
- '',
- 'null',
- 'false',
- 'true',
- '"quack quack I am a duck lol"',
- '{',
- '[',
- '{"foo":',
- '{"foo":"bar",}',
- '{{}',
+ '{' => array(
+ 'line' => 1,
+ 'char' => 1,
+ 'token' => 'EOF',
+ ),
+ '[' => array(
+ 'line' => 1,
+ 'char' => 1,
+ 'token' => 'EOF',
+ ),
+ '{"foo":' => array(
+ 'line' => 1,
+ 'char' => 7,
+ 'token' => 'EOF',
+ ),
+ '{"foo":"bar",}' => array(
+ 'line' => 1,
+ 'char' => 13,
+ 'token' => '}',
+ ),
+ '{{}' => array(
+ 'line' => 1,
+ 'char' => 1,
+ 'token' => '{',
+ ),
+ '{}}' => array(
+ 'line' => 1,
+ 'char' => 2,
+ 'token' => '}',
+ ),
+ "{\"foo\":\"bar\",\n\"bar\":\"baz\",}" => array(
+ 'line' => 2,
+ 'char' => 12,
+ 'token' => '}',
+ ),
+ "{'foo': 'bar'}" => array(
+ 'line' => 1,
+ 'char' => 1,
+ 'token' => 'INVALID',
+ ),
);
- foreach ($tests as $input) {
+ foreach ($tests as $input => $expected) {
+ $caught = null;
+ try {
+ $parser->parse($input);
+ } catch (Exception $ex) {
+ $caught = $ex;
+ }
+ $this->assertTrue($caught instanceof PhutilJSONParserException);
+ $this->assertEqual($expected['line'], $caught->getSourceLine());
+ $this->assertEqual($expected['char'], $caught->getSourceChar());
+ $this->assertEqual($expected['token'], $caught->getSourceToken());
+ }
+ }
+
+ public function testDuplicateKeys() {
+ $parser = new PhutilJSONParser();
+
+ $tests = array(
+ '{"foo": "bar", "foo": "baz"}' => array('foo' => 'baz'),
+ );
+
+ foreach ($tests as $input => $expect) {
+ $parser->setAllowDuplicateKeys(true);
+ $this->assertEqual(
+ $expect,
+ $parser->parse($input),
+ 'Parsing JSON: '.$input);
+
+ $parser->setAllowDuplicateKeys(false);
$caught = null;
try {
$parser->parse($input);
diff --git a/src/parser/exception/PhutilJSONParserException.php b/src/parser/exception/PhutilJSONParserException.php
--- a/src/parser/exception/PhutilJSONParserException.php
+++ b/src/parser/exception/PhutilJSONParserException.php
@@ -1,3 +1,41 @@
<?php
-final class PhutilJSONParserException extends Exception {}
+final class PhutilJSONParserException extends Exception {
+
+ private $sourceLine;
+ private $sourceChar;
+ private $sourceToken;
+ private $expected;
+
+ public function __construct(
+ $message,
+ $line = null,
+ $char = null,
+ $token = null,
+ $expected = null) {
+
+ $this->sourceLine = $line;
+ $this->sourceChar = $char;
+ $this->sourceToken = $token;
+ $this->expected = $expected;
+
+ parent::__construct($message);
+ }
+
+ public function getSourceLine() {
+ return $this->sourceLine;
+ }
+
+ public function getSourceChar() {
+ return $this->sourceChar;
+ }
+
+ public function getSourceToken() {
+ return $this->sourceToken;
+ }
+
+ public function getExpectedTokens() {
+ return $this->expected;
+ }
+
+}
diff --git a/src/utils/__tests__/PhutilUtilsTestCase.php b/src/utils/__tests__/PhutilUtilsTestCase.php
--- a/src/utils/__tests__/PhutilUtilsTestCase.php
+++ b/src/utils/__tests__/PhutilUtilsTestCase.php
@@ -513,24 +513,35 @@
}
public function testPhutilJSONDecode() {
- $default = (object)array();
-
- $cases = array(
+ $valid_cases = array(
'{}' => array(),
'[]' => array(),
- '' => $default,
- '"a"' => $default,
- '{,}' => $default,
- 'null' => $default,
- '"null"' => $default,
'[1, 2]' => array(1, 2),
'{"a":"b"}' => array('a' => 'b'),
);
- foreach ($cases as $input => $expect) {
- $result = phutil_json_decode($input, $default);
+ foreach ($valid_cases as $input => $expect) {
+ $result = phutil_json_decode($input);
$this->assertEqual($expect, $result, 'phutil_json_decode('.$input.')');
}
+
+ $invalid_cases = array(
+ '',
+ '"a"',
+ '{,}',
+ 'null',
+ '"null"',
+ );
+
+ foreach ($invalid_cases as $input) {
+ $caught = null;
+ try {
+ phutil_json_decode($input);
+ } catch (Exception $ex) {
+ $caught = $ex;
+ }
+ $this->assertTrue($caught instanceof PhutilJSONParserException);
+ }
}
public function testCensorCredentials() {
diff --git a/src/utils/utils.php b/src/utils/utils.php
--- a/src/utils/utils.php
+++ b/src/utils/utils.php
@@ -1046,21 +1046,24 @@
/**
- * Decode a JSON dictionary, or return a default value if the input does not
- * decode or does not decode into a dictionary.
+ * Decode a JSON dictionary.
*
* @param string A string which ostensibly contains a JSON-encoded list or
* dictionary.
- * @param default? Optional default value to return if the string does not
- * decode, or does not decode into a list or dictionary.
- * @return mixed Decoded list/dictionary, or default value if string
- * failed to decode.
+ * @return mixed Decoded list/dictionary.
*/
-function phutil_json_decode($string, $default = array()) {
+function phutil_json_decode($string) {
$result = @json_decode($string, true);
+
+ // TODO: Maybe call `json_last_error` if possible...
+
if (!is_array($result)) {
- return $default;
+ // Failed to decode the JSON. Try to use @{class:PhutilJSONParser} instead.
+ // This will probably fail, but will throw a useful exception.
+ $parser = new PhutilJSONParser();
+ $result = $parser->parse($string);
}
+
return $result;
}

File Metadata

Mime Type
text/plain
Expires
Tue, Mar 18, 12:09 AM (1 w, 1 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7340161
Default Alt Text
D9634.id23120.diff (8 KB)

Event Timeline

Phabricator · Built by Phacility