Open GoogleCodeExporter opened 8 years ago
Well, discussing with Jacob now - I think using the PHP type is a bit hazardous
in
terms of security.
There is a flag to pass to json_decode to get it to emit all arrays - otherwise
we
should try to fix upstream in Solr so that it emits serialized objects.
Original comment by pwola...@gmail.com
on 9 Mar 2009 at 9:10
Added another patch. This sets the default back to json and also includes a
fall
back library for < 5.2 PHP installs.
Original comment by jacobsi...@gmail.com
on 9 Mar 2009 at 9:26
Attachments:
There seem to be a number of whitespace issues with the patch (tab vs space).
An alternative to that class would be the (likely better supported) Zend class:
http://framework.zend.com/manual/en/zend.json.html
However, I'm not sure in either case that one should include the external
library
with into this one - a PHP lib should be a last resort for those on PHP 5.1 who
cannot install the PECL extension.
Rather than coercing the PHP response into objects, the extra param could be
based to
json_decode() to make it return arrays.
Original comment by pwola...@gmail.com
on 10 Mar 2009 at 12:10
The Zend class is fine. It doesn't really matter to me. We just need some
type of
backup which works and quickly. This one seems to. If you feel so called to
figure
out the Zend plugin / licensing, go ahead but it seems like not an optimal use
of time.
I feel we should bundle the backup along with the JSON writer. That's pretty
much
where it belongs. It's not a huge crime.
I don't like the json_decode issue because it breaks everyone's client code who
is
using this module. I feel like BC is important and this is not worth
introducing risk.
Original comment by jacobsi...@gmail.com
on 10 Mar 2009 at 1:13
here's a simple patch to require the Zend class if using PHP 5.1 without the
PECL
extension. Not actually tested yet, since I have PHP 5.2, nor am I even sure
this
should be committed - PHP 5.1 users could just patch or do this themselves.
Original comment by pwola...@gmail.com
on 10 Mar 2009 at 10:17
Attachments:
I *believe* Zend_Json_Decoder will use json_decode if it exists, so the
Zend_Json patch would create infinite
recursion.
As for the plugable parsers, that was actually how the client was originally
written. I simplified it to just JSON
after feedback from the Solr developers (Yonik and Grant). I'd prefer to not
have to support more than one
format. As for whether that format will be PHP, Serialized PHP, or JSON should
be based on the native parsing
speed of each and greatest overall support (across PHP versions and Solr
versions).
Keep in mind also that the inclusion of json_decode might not be the only
function in the code that requires
PHP >= 5.2. I will have to run through it and check, but if there are other
functions that require a PHP 5
version then the point of using PHP result formats for 4.3 compatibility may be
moot. They should only be
considered if they can marshalled by the server and parsed by the client faster
than JSON responses.
Original comment by donovan....@gmail.com
on 13 Mar 2009 at 10:56
Hi Donovan,
The only way we can harness our user base (which does include some 5.1 people)
to
test it is to apply a json compat layer.
Whatever we provide, it would be nice to bundle it with the library for
usability
purposes. Since the Zend Framework is BSD, this should be fine, right?
Best,
Jacob
Original comment by jacobsi...@gmail.com
on 18 Mar 2009 at 6:22
Original issue reported on code.google.com by
jacobsi...@gmail.com
on 9 Mar 2009 at 9:01Attachments: