JSON extension dependency has a non-free component: JSON_parser.c
Closed, ResolvedPublic

Description

Since gerrit 50140 was merged, MediaWiki has required the JSON PHP extension
to run. Unfortunately, I just found out the extension has a component with
a license that the Free Software Foundation considers to be non-free,
even though the extension is one of those bundled with PHP (and would
presumably be covered by the PHP License, which is a free license).

https://github.com/php/php-src/blob/master/ext/json/JSON_parser.c#L18
https://bugs.php.net/bug.php?id=63520
https://www.gnu.org/licenses/license-list.html#JSON

Until such time a version of the native JSON extension exists that does
not depend on any non-free component, MediaWiki should continue to provide
a pure PHP fallback under the terms of the GPL or another free license, or
at least make one easy to use if necessary.

Ideally, the fallback should implement the underlying json_encode and
json_decode functions, so MediaWiki extensions that call them directly
will work as intended.


Version: 1.22.0
Severity: blocker
See Also:
https://bugzilla.wikimedia.org/show_bug.cgi?id=47356
https://bugzilla.wikimedia.org/show_bug.cgi?id=26818
https://bugzilla.wikimedia.org/show_bug.cgi?id=54774

bzimport set Reference to bz47431.
PleaseStand created this task.Via LegacyApr 19 2013, 9:41 PM
gerritbot added a comment.Via ConduitApr 19 2013, 9:55 PM

Related URL: https://gerrit.wikimedia.org/r/60080 (Gerrit Change Id3b88102e768318e3605a19e9952121091a40915)

Parent5446 added a comment.Via ConduitApr 20 2013, 11:23 PM

God help me. Just when I thought the Services_JSON class was gone for good...

PleaseStand added a comment.Via ConduitApr 22 2013, 10:16 AM

(In reply to comment #2)

God help me. Just when I thought the Services_JSON class was gone for good...

I do of course agree with the "upstream" classification. MediaWiki shouldn't
have to work around the problem, but rather PHP should be fixed. The main
reason I reverted the changes on REL1_21 was to avoid a worst-case scenario
in which the problem would remain uncorrected, and in response, Debian
and Fedora would start compiling PHP without JSON support, and MediaWiki
1.21 would not work for users of these Linux distributions.

(In reply to comment #0)

Until such time a version of the native JSON extension exists that does
not depend on any non-free component, MediaWiki should continue to provide
a pure PHP fallback under the terms of the GPL or another free license, or
at least make one easy to use if necessary.

Reverting the change on REL1_21 means we have six months to either fix the
problem upstream or work around the problem in MediaWiki. I'm investigating
the former: the feasibility of replacing Crockford's JSON_checker with
a different library.

PleaseStand added a comment.Via ConduitApr 22 2013, 10:27 PM

It appears that the assignee listed on the PHP bug report (remi) has already
forked the JSON extension; asked for a status update on the report.

Krinkle added a comment.Via ConduitApr 29 2013, 9:09 PM

Marking as blocker for the 1.22 release. Though that is still various months away, we can't risk releasing a MediaWiki version that does not work on a PHP version compiled by Debian or Fedora.

So before 1.22 we need to either revert this whole stack (like we did for 1.21, but then in master + 1.22) or ship with free polyfills for it.

PleaseStand added a comment.Via ConduitApr 29 2013, 9:34 PM

Krinkle: I have been working on a "polyfill" to emulate all PHP 5.5 JSON features as accurately as possible while exceeding the performance of Services_JSON.

So far, I have an implementation of json_encode() that seems to work OK, although I shall perform more extensive testing. I'll see if I can start
working on the decoder this week.

Parent5446 added a comment.Via ConduitApr 29 2013, 9:36 PM

The JSON license doesn't seem like it has a copyleft. We could just fork the code and re-license it without the good/evil clause.

PleaseStand added a comment.Via ConduitApr 30 2013, 12:47 AM

(In reply to comment #7)

The JSON license doesn't seem like it has a copyleft. We could just fork the
code and re-license it without the good/evil clause.

[This is exactly what Remi has been working on.][1]

As I noted above, he is attempting to replace the modified version of
Crockford's JSON_checker (the original was merely a JSON validator) with
the [JSON-C library][2].

The remaining portion of the JSON extension (most of the encoding code)
can be kept.

[1]: https://github.com/remicollet/pecl-json-c
[2]: https://github.com/json-c/json-c

gerritbot added a comment.Via ConduitMay 20 2013, 5:55 AM

Related URL: https://gerrit.wikimedia.org/r/64549 (Gerrit Change I11b917a371e07267dfa98b8449776d0c1cb29b15)

PleaseStand added a comment.Via ConduitJun 20 2013, 1:35 AM

(In reply to comment #6)

Krinkle: I have been working on a "polyfill" to emulate all PHP 5.5 JSON
features as accurately as possible while exceeding the performance of
Services_JSON.

So far, I have an implementation of json_encode() that seems to work OK,
although I shall perform more extensive testing. I'll see if I can start
working on the decoder this week.

Debian has now packaged PECL jsonc under the php5-json package name (which
used to be a "virtual package" pointing to php5-common). Right now, the new
package is only in sid (unstable) and is not part of a released version of
Debian. I have not yet tested it with MediaWiki.

http://packages.debian.org/sid/php5-json

JsonFallback status update: I have been working on the tokenization and
string unescaping routines, and hopefully, the decoder will be ready
sometime next week.

PleaseStand added a comment.Via ConduitJun 21 2013, 5:23 PM
  • Bug 49122 has been marked as a duplicate of this bug. ***
gerritbot added a comment.Via ConduitAug 13 2013, 7:44 AM

Change 78941 had a related patch set uploaded by PleaseStand:
Introducing JsonFallback

https://gerrit.wikimedia.org/r/78941

Aklapper added a comment.Via ConduitSep 26 2013, 2:40 PM

The first two patches got merged, the third patch in https://gerrit.wikimedia.org/r/#/c/78941/ still needs reviews.

tstarling added a comment.Via ConduitSep 27 2013, 6:36 AM

From Remi's comment on the PHP bug, I understand that Debian will configure PHP with --disable-json, and distribute jsonc separately. So if MW is going to run out of the box on a plain Debian PHP package, we will need a pure-PHP implementation.

ori added a comment.Via ConduitSep 27 2013, 6:58 AM

(In reply to comment #14)

From Remi's comment on the PHP bug, I understand that Debian will configure
PHP with --disable-json, and distribute jsonc separately. So if MW is going
to run out of the box on a plain Debian PHP package, we will need a pure-PHP
implementation.

Why not have MediaWiki depend on php5-json? My understanding is that php5-json is both properly free and packaged for all versions of Debian that will configure PHP without JSON support.

Anomie added a comment.Via ConduitSep 27 2013, 3:16 PM

(In reply to comment #15)

(In reply to comment #14)
> From Remi's comment on the PHP bug, I understand that Debian will configure
> PHP with --disable-json, and distribute jsonc separately. So if MW is going
> to run out of the box on a plain Debian PHP package, we will need a pure-PHP
> implementation.

Why not have MediaWiki depend on php5-json? My understanding is that
php5-json
is both properly free and packaged for all versions of Debian that will
configure PHP without JSON support.

I've been wondering this as well. I note that a Debian or Ubuntu user is likely to have to install at least php5-mysql or another such package to have database support. We could as easily mention the requirement for php5-json at the same time.

PleaseStand added a comment.Via ConduitSep 27 2013, 5:44 PM

(In reply to comment #16)

I've been wondering this as well. I note that a Debian or Ubuntu user is
likely
to have to install at least php5-mysql or another such package to have
database
support. We could as easily mention the requirement for php5-json at the same
time.

If we do decide to not include a pure-PHP implementation, I will have to make two small changes to the installer, though that shouldn't be a big deal at all.

(a) Adding a new environmental check for json_encode() or json_decode()
(b) Removing one of the two uses of Xml::encodeJsVar. This is for JS encoding an array of database types that doesn't seem to be used anywhere, and which in any case should be alphanumeric (thus simple to encode).

I'll also note that compatibility code does not necessarily have to be included in core MediaWiki, if loading extension code early in the startup process (including the installation process) is allowed for.

tstarling added a comment.Via ConduitSep 30 2013, 12:56 AM

Requiring php5-json and avoiding the reintroduction of a pure-PHP JSON parser would be fine with me, but it would be a shame to see PleaseStand's improved JSON implementation go to waste. So ideally, I would like it to be submitted to PEAR.

gerritbot added a comment.Via ConduitSep 30 2013, 1:09 AM

Change 78941 abandoned by PleaseStand:
Introducing JsonFallback

Reason:
Per bug 47431 comment 18, this code is not going to be maintained as part of MediaWiki. Reviewers, thank you very much for your comments.

https://gerrit.wikimedia.org/r/78941

PleaseStand added a comment.Via ConduitSep 30 2013, 1:14 AM

(In reply to comment #18)

Requiring php5-json and avoiding the reintroduction of a pure-PHP JSON parser
would be fine with me, but it would be a shame to see PleaseStand's improved
JSON implementation go to waste. So ideally, I would like it to be submitted
to
PEAR.

And thus I am closing this RESOLVED FIXED, as having been fixed by Debian. I will shortly open a bug to track the installer changes necessary to ensure the package is not missing.

PleaseStand added a comment.Via ConduitSep 30 2013, 1:30 AM

(In reply to comment #20)

installer changes necessary to ensure the
package is not missing.

bug 54774

PleaseStand added a comment.Via ConduitSep 30 2013, 6:17 PM

I have updated https://www.mediawiki.org/wiki/MediaWiki_1.22 and
the installer (in gerrit 86696).

However, something that slightly worries me is that according to
http://packages.ubuntu.com/saucy/php5-json, php-json (pecl-json-c)
is in "universe" (i.e. is not an officially supported Ubuntu package).
That means Canonical won't necessarily issue security updates for it.

https://wiki.ubuntu.com/SecurityTeam/UpdateProcedures#Responsibility

Reedy added a comment.Via ConduitOct 17 2013, 5:45 PM

For anyone that finds this to be an issue with Ubuntu 13.10, the solution is indeed to do apt-get install php5-json...

Add Comment

Column Prototype
This is a very early prototype of a persistent column. It is not expected to work yet, and leaving it open will activate other new features which will break things. Press "\" (backslash) on your keyboard to close it now.