ietf
[Top] [All Lists]

Re: [apps-discuss] Last Call: <draft-ietf-appsawg-json-pointer-07.txt> (JSON Pointer) to Proposed Standard

2012-12-17 09:09:47
On Mon, Dec 17, 2012 at 2:01 AM, Robert Sayre <sayrer(_at_)gmail(_dot_)com> 
wrote:

[snip]

You might argue that someone who (for example) adds to "/foo/1" in the
mistaken belief that it's an array, when in fact it's an object, will get
surprising results. That's true, but if we were to solve this problem, that
person would still need to understand the underlying semantics of "foo" to
do anything useful to it -- and I'm not hearing anyone complain about that
(I hope).

The first case that can come up is editing documents that have been
refactored. It's quite common to refactor JSON documents as below when
a little extra metadata is needed for a collection:

{ "foo":[1,2,3] }

then becomes

{
  "foo":
  "bar": "baz",
  "members": [1,2,3]
}

now, an operation on /foo/1 is quite a different thing in the second
document. It's not too hard to see how a client that's been out of
contact with the server for a while could send an addition to /foo
intended for the array that's been moved down to "members". Now, there
are ways to catch this if you're willing to bloat the patch document
with other checks, or by insisting on a matching ETag. But one of the
nice things about patch formats is that you can try for more
optimistic concurrent editing. This ambiguity in the patch format
makes it needlessly more costly to detect unsound edits.


Appreciate the concrete use case... while it has come up a few times, this
is the first actual case that's been presented. If I understand the issue
correctly, you're basically talking about a client making a blind partial
update to an arbitrarily structured JSON document whose structure may have
changed since the last time the client edited it...  In such cases, I would
argue that you get whatever you end up with.

Everyone's idea of "bloat" is different. Adding etag checking is a simple,
effective best practice for detecting concurrent document changes and is
likely a really good idea in a scenario where we're making partial
modifications to arbitrary documents. Adding in support for json-predicates
adds a bit more (not a lot really but opinions vary.. in the ruby
implementation it's only approximately 140 more lines of code, mostly
formatting) but can make it rather more reliable... For instance,

  [
    {"op": "type", "path": "/foo/1", "value": "array"},
    {"op": "add", "path": "/foo/1", "value": 1}
  ]

Again, the point is, I don't see this as a problem in practice.
Implementers that make blind edits to arbitrary docs can expect surprising
results sometimes. That's just the way it is. There are mechanisms defined
(test, json-predicates and conditional requests) that can make the edit
more reliable.


Another source of bugs from JavaScript side of things will be
"array-like" objects. These are really quite common (jQuery lists,
arguments object, etc), and authors very often don't realize they
won't be serialized as an array. To be sure, this is a bug on the
author's side, but some checks in the patch format would really help.
They'll probably become even more common as ES6 is standardizing
iterators http://wiki.ecmascript.org/doku.php?id=harmony:iterators.
Here's an example in a JS REPL using the arguments object (but keep in
mind there are many other objects out there that behave this way).

function foo(list) { console.log(JSON.stringify(list)) }
function bar(a,b,c) { foo(arguments) }
bar("a", "b", "c");
{"0":"a","1":"b","2":"c"}


Definitely understand this case, but json-patch is defined in terms of the
basic json model and not javascript. In other words... again.. the
developer using the patch needs to be aware of the structure of the target
resource. Json-predicate provides a good approach to dealing with that by
allowing a developer to test the structure before attempting to edit.

- James


Put another way -- do you really think

Yes, really.

that people PATCHing something as if it's an array (when in fact it's an
object) is a significant, real-world problem, given that the patch author
already has to understand the semantics of the document they're patching?

I don't see that requirement in the draft, and I certainly wouldn't
bet on it being true in all cases. Wasn't one of the motivating
use-cases for this patch format CouchDB? That software operates on
arbitrary, user-defined JSON documents.

- Rob

I don't, and the WG didn't either.

Regards,


On 17/12/2012, at 3:36 PM, Robert Sayre <sayrer(_at_)gmail(_dot_)com> wrote:

The cost of fixing it seems low, either by changing the path syntax of
JSON pointer or changing the names of operations applied to arrays.
Array-like objects are common enough in JavaScript to make this a
worry. The other suggestions either assume a particular policy for
concurrent edits or require more machinery (test operation etc).
Wouldn't it be simpler to make the patch format more precise?

- Rob

On Sun, Dec 16, 2012 at 4:33 PM, Matthew Morley <matt(_at_)mpcm(_dot_)com> 
wrote:
I am usually lurking and struggling to keep up with these posts. But, I
concur with James, this really is a non-issue in practice.

The JSON Pointer expresses a path down a JSON object to a specific
context.
The Patch expresses a change within or to that context.
Everything about the both standards is about that end context.

If you want to confirm the type of the context before applying a
patch, this
should probably be part of a test operation. I'm not sure if this is
possible at this point (?), but that is where the logic should exist.



On Sun, Dec 16, 2012 at 12:22 AM, James M Snell 
<jasnell(_at_)gmail(_dot_)com>
wrote:




On Sat, Dec 15, 2012 at 8:36 PM, Robert Sayre 
<sayrer(_at_)gmail(_dot_)com>
wrote:

On Fri, Dec 14, 2012 at 9:17 AM, Markus Lanthaler
<markus(_dot_)lanthaler(_at_)gmx(_dot_)net> wrote:

Hmm.. I think that’s quite problematic. Especially considering how
JSON
Pointer is used in JSON Patch.

I agree--I provided the same feedback privately. It seems
straightforwardly unsound.


In practice it doesn't seem to be much of an issue.

Specifically, if I GET an existing document and get an etag with the
JSON,
then make some changes and send a PATCH with If-Match, the fact that
any
given pointer could point to an array or object member doesn't really
matter
much.

For example:

GET /the/doc HTTP/1.1

 <  HTTP/1.1 200 OK
    ETag: "my-document-tag"
    Content-Type: application/json

    {"1":"foo"}

PATCH /the/doc HTTP/1.1
    If-Match: "my-document-etag"
    Content-Type: application/json-patch

    [{"op":"add","path":"/2","value":"bar"}]

Generally speaking, someone should not be using PATCH to perform a
partial
modification if they don't already have some knowledge in advance
what they
are modifying. The only time the apparent ambiguity becomes an issue
is when
a client is blindly sending a patch to an unknown endpoint... in
which case,
you get whatever you end up with.

- James



- Rob



--

Markus Lanthaler

@markuslanthaler







From: James M Snell [mailto:jasnell(_at_)gmail(_dot_)com]
Sent: Friday, December 14, 2012 5:41 PM
To: Markus Lanthaler
Cc: IETF Discussion; IETF Apps Discuss
Subject: Re: [apps-discuss] Last Call:
<draft-ietf-appsawg-json-pointer-07.txt> (JSON Pointer) to Proposed
Standard



JSON Pointer does not distinguish between objects and arrays. That
is
not determined until the pointer is applied to an actual object
instance...
the pointer "/1" is valid against {"1":"a"} or ["a","b"]



On Fri, Dec 14, 2012 at 2:51 AM, Markus Lanthaler
<markus(_dot_)lanthaler(_at_)gmx(_dot_)net> wrote:

I've asked that before but didn't get an answer. So let me ask again
(even
though I'm quite sure it has already been asked by somebody else).

How does JSON Pointer distinguish between objects and arrays? E.g.
consider
the following JSON document:

{
 "foo": "bar",
 "1": "baz"
}

As I read the draft, the JSON Pointer "/1" would evaluate to "baz"
even
though that's probably not what the author intended. Is there a way
to
avoid
that?


Thanks,
Markus



--
Markus Lanthaler
@markuslanthaler





-----Original Message-----
From: apps-discuss-bounces(_at_)ietf(_dot_)org [mailto:apps-discuss-
bounces(_at_)ietf(_dot_)org] On Behalf Of The IESG
Sent: Tuesday, December 11, 2012 4:01 PM
To: IETF-Announce
Cc: apps-discuss(_at_)ietf(_dot_)org
Subject: [apps-discuss] Last Call:
<draft-ietf-appsawg-json-pointer-
07.txt> (JSON Pointer) to Proposed Standard


The IESG has received a request from the Applications Area Working
Group
WG (appsawg) to consider the following document:
- 'JSON Pointer'
 <draft-ietf-appsawg-json-pointer-07.txt> as Proposed Standard

The IESG plans to make a decision in the next few weeks, and
solicits
final comments on this action. Please send substantive comments to
the
ietf(_at_)ietf(_dot_)org mailing lists by 2012-12-25. Exceptionally, 
comments
may
be
sent to iesg(_at_)ietf(_dot_)org instead. In either case, please 
retain the
beginning of the Subject line to allow automated sorting.

Abstract


  JSON Pointer defines a string syntax for identifying a specific
value
  within a JSON document.




The file can be obtained via
http://datatracker.ietf.org/doc/draft-ietf-appsawg-json-pointer/

IESG discussion can be tracked via


http://datatracker.ietf.org/doc/draft-ietf-appsawg-json-pointer/ballot/


No IPR declarations have been submitted directly on this I-D.


_______________________________________________
apps-discuss mailing list
apps-discuss(_at_)ietf(_dot_)org
https://www.ietf.org/mailman/listinfo/apps-discuss

_______________________________________________
apps-discuss mailing list
apps-discuss(_at_)ietf(_dot_)org
https://www.ietf.org/mailman/listinfo/apps-discuss




_______________________________________________
apps-discuss mailing list
apps-discuss(_at_)ietf(_dot_)org
https://www.ietf.org/mailman/listinfo/apps-discuss




_______________________________________________
apps-discuss mailing list
apps-discuss(_at_)ietf(_dot_)org
https://www.ietf.org/mailman/listinfo/apps-discuss




--
Matthew P. C. Morley
_______________________________________________
apps-discuss mailing list
apps-discuss(_at_)ietf(_dot_)org
https://www.ietf.org/mailman/listinfo/apps-discuss

--
Mark Nottingham   http://www.mnot.net/



_______________________________________________
apps-discuss mailing list
apps-discuss(_at_)ietf(_dot_)org
https://www.ietf.org/mailman/listinfo/apps-discuss

<Prev in Thread] Current Thread [Next in Thread>