Last modified: 2009-09-09 22:43:01 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T22336, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 20336 - Don't assume json availability
Don't assume json availability
Status: RESOLVED FIXED
Product: MediaWiki
Classification: Unclassified
General/Unknown (Other open bugs)
1.16.x
All All
: Normal major (vote)
: ---
Assigned To: Michael Dale
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-08-21 14:39 UTC by Platonides
Modified: 2009-09-09 22:43 UTC (History)
4 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description Platonides 2009-08-21 14:39:00 UTC
ForeignAPIRepo and mwEmbed make use of json_decode, but that's only available with PHP 5 >= 5.2.0 (or PECL json extension), conflicting with the requeriment of just PHP 5.
Those cases should use the implementation from include/api/ApiFormatJson_json.php if native json_decode isn't available.

Greg Sabino Mullani reports that not having json (default CentOS install) produces 
PHP Fatal error:  Call to undefined function json_decode() in /var/www/html/bucardo.org/mwp3/js2/mwEmbed/php/jsAutoloadLocalClasses.php on line 23
Comment 1 Roan Kattouw 2009-08-21 14:46:20 UTC
An easy fix (and workaround, paste this code in LocalSettings.php):
if(!function_exists('json_decode')) {
        function json_decode($val) {
                $json = new Services_JSON(SERVICES_JSON_LOOSE_TYPE);
                return $json->decode($val);
        }
}
Comment 2 Greg Sabino Mullane 2009-08-21 15:08:33 UTC
Here's what I did on CentOS to get json installed, for the record:

* yum install php-devel gcc
* wget http://pecl.php.net/get/json
* tar xvfz json-1.2.1.tgz
* cd json-1.2.1
* phpize
* ./configure
* make install
* Edit php.ini, add extension=json.so to the extensions section
* service httpd restart
Comment 3 Chad H. 2009-08-21 18:56:05 UTC
(In reply to comment #1)
> An easy fix (and workaround, paste this code in LocalSettings.php):
> if(!function_exists('json_decode')) {
>         function json_decode($val) {
>                 $json = new Services_JSON(SERVICES_JSON_LOOSE_TYPE);
>                 return $json->decode($val);
>         }
> }
> 

This would be a very good way to go about this, similar to what we do iconv, mb_strlen, etc.
Comment 4 Brion Vibber 2009-08-23 01:06:57 UTC
We've got a fair number of places using ApiFormatJson::getJsonEncode() right now (a wrapper which checks if json_encode is present and non-buggy, then runs either through json_encode or the Services_JSON class) but there's no equivalent wrapper for decoding.

I would probably recommend breaking our Services_JSON copy out of the api dir and have nice global wrapper methods for both encoding and decoding which aren't API-specific...

(Using a class static method is friendly to the autoloader and means we don't have to worry about defining the function if we're not going to use it; it also avoids worrying about whether we're going to break something else that sees our fake json_encode() and thinks that something else is available which isn't.)

Note that our preexisting Xml::encodeJsVar() is basically a simple JSON encoder as well (though currently it produces extra spacing and doesn't handle associative arrays right). We should probably merge some of these things together...

Xml::jsonEncode() and Xml::jsonDecode() maybe? Or we could break them out to a new class with a nice clean name... FormatJson::encode() and FormatJson::decode() ?
Comment 5 Michael Dale 2009-09-09 22:43:01 UTC
Oky broken out into the global functions file simple class with static methods FormatJson::encode() and FormatJson::decode() replaced instances where json availability was assumed or was calling the static ApiFormatJson method. in r56116

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links