New json.parse issues

[Cassie <do...@apache.org>]

Kevin -

Your newly checked in json.parse is giving me grief. It says this is valid
json:
"{3 : 5}"

while these are not:
"{'3' : '5'}"
"{x : y}"


The first thing is that is requires all keys to -not- be escaped. If the
json object has a single quote in it, it will fail. Secondly, your object
can only have numbers in it, not text. I'm not exactly sure on how to change
that crazy regex without breaking something you wanted to catch. (I mean it
is easy to add single quote to the line on 156.. but is that ok?)

Thanks for looking into it!

- Cassie


ps - This is definitely a place where a couple of js tests would make a huge
difference.

Re: New json.parse issues

[Kevin Brown <et...@google.com>]

I've noticed that firebug doesn't always display uncaught exceptions; for
the time being we should probably make it so that parse always returns a
false equivalent (false, null, whatever) so it can be used as:

if (var data = gadgets.json.parse(string)) {
   doSomethingWithMy(data);
} else {
  showAnErrorMessage();
}

On Thu, Mar 6, 2008 at 5:33 AM, Cassie <do...@apache.org> wrote:

> Ha, you are absolutely right. Sorry for the false claim... I completely
> forgot to try switching to double quotes. (I ran into this because it
> broken
> the opensocial-0.7 feature btw)
>
> As a side note, when calling makeRequest with an invalid json response it
> doesn't actually propagate that error you are throwing "throw new
> Error('parseJSON');" (at least in firefox w/firebug)
>
> I have no idea why it isn't getting to the browser nor did I look into it
> much, it was just confusing while debugging.
> Thanks for your help.
>
> - Cassie
>
>
> On Thu, Mar 6, 2008 at 2:22 PM, Kevin Brown <et...@google.com> wrote:
>
> > On Thu, Mar 6, 2008 at 3:36 AM, Cassie <do...@apache.org> wrote:
> >
> > > Kevin -
> > >
> > > Your newly checked in json.parse is giving me grief. It says this is
> > valid
> > > json:
> > > "{3 : 5}"
> > >
> > > while these are not:
> > > "{'3' : '5'}"
> > > "{x : y}"
> >
> >
> > Actually, none of those should be valid JSON according to the spec,
> > although
> > the first form is inherently safe and matches ecmascript 3 anyway so it
> > isn't surprising that the standard json implementation allows it.
> >
> > The previous parser was incorrectly allowing the first of the second two
> > forms, but it turns out that this leaves open an eval exploit wherein
> you
> > can escape the double quotes. Zhen and I actually added this because we
> > thought it was a bogus limitation in the JSON spec, but it turns out to
> be
> > there for good reason. JSON requires all strings to be double quoted.
> > Since
> > this has never been allowed in the JSON spec anyway, it should have
> never
> > been used.
> >
> > JSON allows keys to be strings (always double quoted), and rvalues to be
> > strings, numbers, objects, or arrays. Anything else is not safe to
> eval(),
> > which means that it requires writing a complete JSON parser in
> > javascript...a very slow task.
> >
> > The second form has never been allowed, because it involves variable
> > names.
> >
> >
> > > The first thing is that is requires all keys to -not- be escaped. If
> the
> > > json object has a single quote in it, it will fail.
> >
> >
> > No, it will fail if the string is quoted with single quotes. Use double
> > quotes to quote strings (as per the JSON spec)
> >
> > var str = '{"please don\'t break me big bad json parser" : "ok, I
> > won\'t"}'
> > var obj = gadgets.json.parse(str);
> >
> >
> > > Secondly, your object
> > > can only have numbers in it, not text.
> >
> >
> > Not sure I follow -- keys can be strings (and apparently numbers for
> some
> > reason), and values can be numbers, strings, objects, or arrays. strings
> > must always be quoted because eval would interpret the variables, and
> that
> > would be bad.
> >
>



-- 
~Kevin

Re: New json.parse issues

[Cassie <do...@apache.org>]

Ha, you are absolutely right. Sorry for the false claim... I completely
forgot to try switching to double quotes. (I ran into this because it broken
the opensocial-0.7 feature btw)

As a side note, when calling makeRequest with an invalid json response it
doesn't actually propagate that error you are throwing "throw new
Error('parseJSON');" (at least in firefox w/firebug)

I have no idea why it isn't getting to the browser nor did I look into it
much, it was just confusing while debugging.
Thanks for your help.

- Cassie


On Thu, Mar 6, 2008 at 2:22 PM, Kevin Brown <et...@google.com> wrote:

> On Thu, Mar 6, 2008 at 3:36 AM, Cassie <do...@apache.org> wrote:
>
> > Kevin -
> >
> > Your newly checked in json.parse is giving me grief. It says this is
> valid
> > json:
> > "{3 : 5}"
> >
> > while these are not:
> > "{'3' : '5'}"
> > "{x : y}"
>
>
> Actually, none of those should be valid JSON according to the spec,
> although
> the first form is inherently safe and matches ecmascript 3 anyway so it
> isn't surprising that the standard json implementation allows it.
>
> The previous parser was incorrectly allowing the first of the second two
> forms, but it turns out that this leaves open an eval exploit wherein you
> can escape the double quotes. Zhen and I actually added this because we
> thought it was a bogus limitation in the JSON spec, but it turns out to be
> there for good reason. JSON requires all strings to be double quoted.
> Since
> this has never been allowed in the JSON spec anyway, it should have never
> been used.
>
> JSON allows keys to be strings (always double quoted), and rvalues to be
> strings, numbers, objects, or arrays. Anything else is not safe to eval(),
> which means that it requires writing a complete JSON parser in
> javascript...a very slow task.
>
> The second form has never been allowed, because it involves variable
> names.
>
>
> > The first thing is that is requires all keys to -not- be escaped. If the
> > json object has a single quote in it, it will fail.
>
>
> No, it will fail if the string is quoted with single quotes. Use double
> quotes to quote strings (as per the JSON spec)
>
> var str = '{"please don\'t break me big bad json parser" : "ok, I
> won\'t"}'
> var obj = gadgets.json.parse(str);
>
>
> > Secondly, your object
> > can only have numbers in it, not text.
>
>
> Not sure I follow -- keys can be strings (and apparently numbers for some
> reason), and values can be numbers, strings, objects, or arrays. strings
> must always be quoted because eval would interpret the variables, and that
> would be bad.
>

Re: New json.parse issues

[Kevin Brown <et...@google.com>]

On Thu, Mar 6, 2008 at 3:36 AM, Cassie <do...@apache.org> wrote:

> Kevin -
>
> Your newly checked in json.parse is giving me grief. It says this is valid
> json:
> "{3 : 5}"
>
> while these are not:
> "{'3' : '5'}"
> "{x : y}"


Actually, none of those should be valid JSON according to the spec, although
the first form is inherently safe and matches ecmascript 3 anyway so it
isn't surprising that the standard json implementation allows it.

The previous parser was incorrectly allowing the first of the second two
forms, but it turns out that this leaves open an eval exploit wherein you
can escape the double quotes. Zhen and I actually added this because we
thought it was a bogus limitation in the JSON spec, but it turns out to be
there for good reason. JSON requires all strings to be double quoted. Since
this has never been allowed in the JSON spec anyway, it should have never
been used.

JSON allows keys to be strings (always double quoted), and rvalues to be
strings, numbers, objects, or arrays. Anything else is not safe to eval(),
which means that it requires writing a complete JSON parser in
javascript...a very slow task.

The second form has never been allowed, because it involves variable names.


> The first thing is that is requires all keys to -not- be escaped. If the
> json object has a single quote in it, it will fail.


No, it will fail if the string is quoted with single quotes. Use double
quotes to quote strings (as per the JSON spec)

var str = '{"please don\'t break me big bad json parser" : "ok, I won\'t"}'
var obj = gadgets.json.parse(str);


> Secondly, your object
> can only have numbers in it, not text.


Not sure I follow -- keys can be strings (and apparently numbers for some
reason), and values can be numbers, strings, objects, or arrays. strings
must always be quoted because eval would interpret the variables, and that
would be bad.