-
Notifications
You must be signed in to change notification settings - Fork 711
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix for issue 1011: add 3rd input encoding parameter to json_encode modifier and let it default to \Smarty\Smarty::$_CHARSET #1016
Open
cmanley
wants to merge
10
commits into
smarty-php:master
Choose a base branch
from
cmanley:master
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
4aedd6b
Add 3rd input encoding parameter to json_encode modifier let it defau…
cmanley f4defd7
Add input_encoding parameter to json_encode modifier documentation
cmanley 2f40db9
Minor optimization of json_encode modifier to skip creating closure i…
cmanley 52878fe
Add unit tests to prove that new json_encode modifier works with spec…
cmanley 77cac4e
Remove requirement to save json_encode modifier cp1252 unit test file…
cmanley e2b1d71
Fix classname of PluginModifierJsonEncodeCp1252Test
wisskid 9316a42
Add tearDown that resets smarty charset to UTF-8 after each test, to …
wisskid ef2fa74
Update DefaultExtension.php
wisskid cbbb244
Move recursive transcoder from json_encode() into src/Extension/Defau…
cmanley 0027e97
Remove debugging
cmanley File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,119 @@ | ||
<?php declare(strict_types=1); | ||
|
||
namespace Smarty\Extension\DefaultExtension; | ||
|
||
use Smarty\Exception; | ||
|
||
class RecursiveTranscoder { | ||
|
||
/** | ||
* Determines if the given value is a possible candidate for transcoding. | ||
* | ||
* @param mixed $value | ||
* @return bool | ||
*/ | ||
public static function is_transcoding_candidate($value): bool { | ||
if (empty($value)) { | ||
return false; | ||
} | ||
return is_string($value) || is_array($value) || is_object($value); | ||
} | ||
|
||
|
||
/** | ||
* Similar to mb_convert_encoding(), but operates recursively on keys and values of arrays, and on objects too. | ||
* Objects implementing \JsonSerializable and unsupported types are returned unchanged. | ||
* The following boolean options/hints are supported (all default to false): | ||
* - ignore_keys: do not transcode array keys | ||
* - ignore_objects: leave objects alone, i.e. do not convert them into associative arrays and transcode them | ||
* - ignore_JsonSerializable_objects: if transcoded result is meant as input for json_encode(), then set this to true. | ||
* | ||
* @param mixed $data | ||
* @param string $to_encoding | ||
* @param string $from_encoding of $data; defaults to \Smarty\Smarty::$_CHARSET | ||
* @param array $options | ||
* @return mixed | ||
*/ | ||
public static function transcode($data, string $to_encoding, string $from_encoding = null, array $options = null) { | ||
if (!static::is_transcoding_candidate($data)) { | ||
return $data; | ||
} | ||
if (!$from_encoding) { | ||
$from_encoding = \Smarty\Smarty::$_CHARSET; | ||
} | ||
if (strcasecmp($to_encoding, $from_encoding) == 0) { | ||
return $data; | ||
} | ||
|
||
# most cases: | ||
if (is_string($data)) { | ||
return mb_convert_encoding($data, $to_encoding, $from_encoding); # string|false | ||
} | ||
|
||
# convert object to array to be transcoded as array | ||
if (is_object($data)) { | ||
if (!empty($options['ignore_objects'])) { | ||
return $data; | ||
} | ||
if (is_a($data, \JsonSerializable::class)) { | ||
if (!empty($options['ignore_JsonSerializable_objects'])) { | ||
return $data; # \JsonSerializable objects should be trusted to serialize themselves into data that can be consumed by json_encode() no matter what the application's default encoding is. | ||
} | ||
} | ||
$data = get_object_vars($data); # public properties as key => value pairs | ||
} | ||
|
||
if (!(is_array($data) && $data)) { | ||
return $data; # any empty array or non-array type as a possible result of object conversion above | ||
} | ||
|
||
# $data is a filled array | ||
$must_transcode_keys = empty($options['ignore_keys']); | ||
$result = $must_transcode_keys ? [] : null; # replacement for $data if keys are transcoded too (i.e. $must_transcode_keys) | ||
$this_func = __FUNCTION__; # for recursion | ||
foreach ($data as $k => &$v) { | ||
if ($must_transcode_keys && is_string($k)) { | ||
$converted_k = mb_convert_encoding($k, $to_encoding, $from_encoding); # string|false | ||
if ($converted_k === false) { # this means mb_convert_encoding() failed which should've triggered a warning | ||
# One of three things can be done here: | ||
# 1. throw an Exception | ||
# 2. return false, indicating to caller that mb_convert_encoding() failed | ||
# 3. do nothing and use the original key | ||
#return false; | ||
throw Exception("Failed to encode array key \"$k\" from $from_encoding to $to_encoding"); | ||
} | ||
else { | ||
$k = $converted_k; | ||
} | ||
} | ||
if (static::is_transcoding_candidate($v)) { | ||
# recurse | ||
$converted_v = static::$this_func($v, $to_encoding, $from_encoding, $options); | ||
if ($converted_v === false) { # this means that $v is a string and that mb_convert_encoding() failed, which should've triggered a warning | ||
# One of four things can be done here: | ||
# 1. throw an Exception | ||
# 2. return false, indicating to caller that mb_convert_encoding() failed | ||
# 3. do nothing and use the original value | ||
# 4. replace the original value with false | ||
#return false; | ||
throw Exception('Failed to encode array value' . (is_string($v) ? " \"$k\"" : '') . 'of type ' . gettype($v) . " from $from_encoding to $to_encoding"); | ||
} | ||
else { | ||
$v = $converted_v; | ||
if ($must_transcode_keys) { | ||
$result[$k] = $v; | ||
} | ||
} | ||
} | ||
else { | ||
# $v may be false here, and in this case it is not an error (since no transcoding occurred since it's not a transcoding candidate) | ||
if ($must_transcode_keys) { | ||
$result[$k] = $v; | ||
} | ||
} | ||
unset($v); | ||
} | ||
return $must_transcode_keys ? $result : $data; | ||
} | ||
|
||
} |
88 changes: 88 additions & 0 deletions
88
...s/UnitTests/TemplateSource/TagTests/PluginModifier/PluginModifierJsonEncodeCp1252Test.php
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,88 @@ | ||
<?php | ||
/** | ||
* Smarty PHPunit tests of modifier. | ||
* This file should be saved in UTF-8 encoding for comment legibility. | ||
*/ | ||
|
||
namespace UnitTests\TemplateSource\TagTests\PluginModifier; | ||
use PHPUnit_Smarty; | ||
|
||
class PluginModifierJsonEncodeCp1252Test extends PHPUnit_Smarty | ||
{ | ||
public function setUp(): void | ||
{ | ||
$this->setUpSmarty(__DIR__); | ||
\Smarty\Smarty::$_CHARSET = 'cp1252'; | ||
} | ||
|
||
public function tearDown(): void | ||
{ | ||
\Smarty\Smarty::$_CHARSET = 'UTF-8'; | ||
} | ||
|
||
/** | ||
* @dataProvider dataForDefault | ||
*/ | ||
public function testDefault($value, $expected) | ||
{ | ||
$tpl = $this->smarty->createTemplate('string:{$v|json_encode}'); | ||
$tpl->assign("v", $value); | ||
$this->assertEquals($expected, $this->smarty->fetch($tpl)); | ||
} | ||
|
||
/** | ||
* @dataProvider dataForDefault | ||
*/ | ||
public function testDefaultAsFunction($value, $expected) | ||
{ | ||
$tpl = $this->smarty->createTemplate('string:{json_encode($v)}'); | ||
$tpl->assign("v", $value); | ||
$this->assertEquals($expected, $this->smarty->fetch($tpl)); | ||
} | ||
|
||
public function dataForDefault() { | ||
$json_serializable_object = new class() implements \JsonSerializable { | ||
public function jsonSerialize(): mixed { | ||
return ["Schl\xC3\xBCssel" => "Stra\xC3\x9Fe"]; # UTF-8 ready for json_encode(); to prove that transcoding doesn't attempt to transcode this again | ||
#return ['Schlüssel' => 'Straße']; # alternatively, this can be used, but then this file must always be saved in UTF-8 encoding or else the test will fail. | ||
} | ||
}; | ||
return [ | ||
["abc", '"abc"'], | ||
[["abc"], '["abc"]'], | ||
[["abc",["a"=>2]], '["abc",{"a":2}]'], | ||
[["\x80uro",["Schl\xFCssel"=>"Stra\xDFe"]], '["\u20acuro",{"Schl\u00fcssel":"Stra\u00dfe"}]'], # x80 = € = euro, xFC = ü = uuml, xDF = ß = szlig | ||
[$json_serializable_object, '{"Schl\u00fcssel":"Stra\u00dfe"}'], | ||
]; | ||
} | ||
|
||
/** | ||
* @dataProvider dataForForceObject | ||
*/ | ||
public function testForceObject($value, $expected) | ||
{ | ||
$tpl = $this->smarty->createTemplate('string:{$v|json_encode:16}'); | ||
$tpl->assign("v", $value); | ||
$this->assertEquals($expected, $this->smarty->fetch($tpl)); | ||
} | ||
|
||
/** | ||
* @dataProvider dataForForceObject | ||
*/ | ||
public function testForceObjectAsFunction($value, $expected) | ||
{ | ||
$tpl = $this->smarty->createTemplate('string:{json_encode($v,16)}'); | ||
$tpl->assign("v", $value); | ||
$this->assertEquals($expected, $this->smarty->fetch($tpl)); | ||
} | ||
|
||
public function dataForForceObject() { | ||
return [ | ||
["abc", '"abc"'], | ||
[["abc"], '{"0":"abc"}'], | ||
[["abc",["a"=>2]], '{"0":"abc","1":{"a":2}}'], | ||
[["\x80uro"], '{"0":"\u20acuro"}'], | ||
]; | ||
} | ||
|
||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My IDE tells me that \json_encode (and \JsonSerializable::class) are not part of the core of PHP prior to PHP8. We should probably check for
function_exists('json_encode')
and do something accordingly.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's strange that someone would want to use the |json_encode modifier without having the json extension installed. But what for exception (and message) to you suggest to throw if it's not present? The check can be done once (using a static boolean var) the first time the modifier is called.