JSONPath: Query expressions for JSON
draft-ietf-jsonpath-base-21

Thank you to Dan Harkins for the SECDIR review.

** Section 4.2.  Recommend unpacking “translated” to be more specific.

OLD
   These variables need to be translated into the
   form they take in a JSONPath query, e.g., by escaping string
   delimiters, or by only allowing specific constructs such as .name to
   be formed when the given values allow that.

NEW
These input variables will need to be validated against application-specific constraints (e.g., only allowing specific constructs such as .name to be formed when the given values allow that) and reformatted (e.g., by escaping string delimiters).

** Section 4.2.  I found the language of “Availability, Confidentiality, and Integrity breaches” generic.  Consider:

OLD
  Failure to perform these
   translations correctly can lead to unexpected failures, which can
   lead to Availability, Confidentiality, and Integrity breaches, in
   particular if an adversary has control over the values (e.g., by
   entering them into a Web form).  The resulting class of attacks,
   _injections_ (e.g., SQL injections), is consistently found among the
   top causes of application security vulnerabilities and requires
   particular attention.

NEW
In certain applications (e.g., a web-form), an adversary might be able to directly control the input into a JSONPath.  Failure to validate and escape input can result in variety of injection attacks whose result could include compromise of the endpoint running the application and arbitrary code execution; corruption or alteration of data assessable to the endpoint (e.g., SQL injection); or altering the security context of execution (e.g., XSS).

Thanks for the hard work put behind this specification. I have not found transport related issues in my review and the overall document looks good to me.

Thanks for the work done in this document. Not easy to herd existing implementations around a specification.

Due to $dayjob and vacations, I only had time for a quick review. No need to reply, but answers will be read with interest.

Hope this helps

-éric

## Figure 1

Just wonder why book is an array and bicycle is not.

## Section 2.3.5.2.2

From my reading of the '<' operator, strings are compared only to unicode ignoring any collation sequence, i.e., "eric" < "zoe" < "éric", which is weird...

Possibly due to my quick browsing through the I-D, but is there a way to make case-insensitive comparaison ? "2001:DB8::A" == "2001:db8::a" ?

# GEN AD review of draft-ietf-jsonpath-base-19

CC @larseggert

Thanks to Linda Dunbar for the General Area Review Team (Gen-ART) review
(https://mailarchive.ietf.org/arch/msg/gen-art/cnQJKq1Dow65ZjgWZxwNUZpVPxE).

## Nits

All comments below are about very minor potential issues that you may choose to
address in some way - or ignore - as you see fit. Some were flagged by
automated tools (via https://github.com/larseggert/ietf-reviewtool), so there
will likely be some false positives. There is no need to let me know what you
did with these suggestions.

### URLs

These URLs in the document can probably be converted to HTTPS:

 * http://www.ecma-international.org/publications/files/ECMA-ST-ARCH/ECMA-262,%203rd%20edition,%20December%201999.pdf

### Grammar/style

#### Section 1.1, paragraph 12
```
recursively. More formally, the descendants relation between nodes is the tr
                                ^^^^^^^^^^^
```
An apostrophe may be missing.

#### Section 1.5, paragraph 3
```
book[?@.isbn] | all books with an ISBN number | +------------------------+---
                                  ^^^^^^^^^^^
```
This phrase is redundant ("N" stands for "number"). Consider using "ISBN".

#### Section 1.5, paragraph 6
```
as a JSONPath query needs to be _well formed_ and _valid_. A string is a well
                                 ^^^^^^^^^^^
```
This word is normally spelled with a hyphen.

#### Section 1.5, paragraph 6
```
error for any query which is not well formed and valid. The well-formedness
                                 ^^^^^^^^^^^
```
This word is normally spelled with a hyphen.

#### Section 2.3.4.2.2, paragraph 9
```
filter expression is composed of side-effect free constituents, the order of
                                 ^^^^^^^^^^^
```
Did you mean "side effect" (=adverse effect, unintended consequence)? Open
compounds are not hyphenated.

#### Section 2.3.5.3, paragraph 11
```
ined such that its evaluation is side-effect free, i.e., all possible orders
                                 ^^^^^^^^^^^
```
Did you mean "side effect" (=adverse effect, unintended consequence)? Open
compounds are not hyphenated.

#### Section 2.3.5.3, paragraph 12
```
n expressions in a query must be well formed (by conforming to the above ABNF
                                 ^^^^^^^^^^^
```
This word is normally spelled with a hyphen.

## Notes

This review is in the ["IETF Comments" Markdown format][ICMF], You can use the
[`ietf-comments` tool][ICT] to automatically convert this review into
individual GitHub issues. Review generated by the [`ietf-reviewtool`][IRT].

[ICMF]: https://github.com/mnot/ietf-comments/blob/main/format.md
[ICT]: https://github.com/mnot/ietf-comments
[IRT]: https://github.com/larseggert/ietf-reviewtool

I've cleared my discuss position based on -20 (and hence not formally blocking), but there are still a few comments that it might be worth a bit more discussion on (via the existing email thread).

Thanks,
Rob

------
Previous comments (unchanged).

Minor level comments:

(4) p 10, sec 1.5.  JSONPath Examples

   Table 2 shows some JSONPath queries that might be applied to this
   example and their intended results.

Here, and in the table title below, it wasn't clear to me why it is "intended results" rather than just "results".


(5) p 12, sec 2.1.  Overview

   2.  Uses of function extensions must be _well typed_, as described in
       Section 2.4.

Isn't this an interop issue, hence shouldn't this be MUST?


(6) p 18, sec 2.3.1.3.  Examples

                      Table 5: Name selector examples

I think that it may be useful to also include an example that returns an object rather than just a value.


(7) p 18, sec 2.3.1.3.  Examples

                      Table 5: Name selector examples

Various table list "Result Paths", but I presume that these are only intended to help make the specification clear, and are not returned (by a standard implementation).  Would this be worth stating or clarifying?


(8) p 21, sec 2.3.3.2.  Semantics

   A negative index-selector counts from the array end.  For example,
   the selector -1 selects the last and the selector -2 selects the
   penultimate element of an array with at least two elements.  As with
   non-negative indexes, it is not an error if such an element does not
   exist; this simply means that no element is selected.

Perhaps "counts back from the array end" rather than "counts from the array end", or possibly it woudld be cleared to say that the absolute position is calculated by summing the length of the array with the negative index.  Otherwise, if counting for the end of the array, it isn't clear to me why -1 doesn't return the penultimate item, other than that not being helpful, of course.


(9) p 28, sec 2.3.5.1.  Syntax

   singular-query      = rel-singular-query / abs-singular-query
   rel-singular-query  = current-node-identifier singular-query-segments
   abs-singular-query  = root-identifier singular-query-segments
   singular-query-segments = *(S (name-segment / index-segment))
   name-segment        = ("[" name-selector "]") /
                         ("." member-name-shorthand)
   index-segment       = "[" index-selector "]"

Minor comment, there is no "intro" text above the "singular-query" block of ABNF so it reads a bit like it is part of the "Comparison expressions" text.


(10) p 29, sec 2.3.5.2.  Semantics

   The filter selector works with arrays and objects exclusively.  Its
   result is a list of _zero_, _one_, _multiple_ or _all_ of their array
   elements or member values, respectively.  Applied to primitive
   values, it selects nothing.

By "selects nothing" this presumably means that it returns an empty nodelist, i.e., and hence the JsonPath expression is likely to return nothing, as opposed to "does nothing" and the it just moves on to the next selector.  I think that the current text is probably obvious enough, but flagging just in case you want to consider rewording it.


(11) p 30, sec 2.3.5.2.1.  Existence Tests

   *  they work with queries that select structured values.
   To examine the value of a node selected by a query, an explicit
   comparison is necessary.  For example, to test whether the node
   selected by the query @.foo has the value null, use @.foo == null
   (see Section 2.6) rather than the negated existence test !@.foo
   (which yields false if @.foo selects a node, regardless of the node's
   value).

I presume that there is no way to change this, but from a correctness perspective, I wish that the existance check had to be explicit, (e.g., perhaps by a function call), to avoid the likely common mistake you describe above, particularly if the test is against a boolean value.  Possibly giving the example for the boolean value case may also be helpful to be explicit here.


(12) p 47, sec 2.5.2.3.  Examples

                   Table 16: Descendant segment examples

The row for "$..[*] $..*" is not clear to me.  I presume that this is meant the same query written two different ways rather than a single query?



Nit level comments:

(13) p 14, sec 2.1.3.  Example

   Consider this example.  With the query argument
   {"a":[{"b":0},{"b":1},{"c":2}]}, the query $.a[*].b selects the
   following list of nodes: 0, 1 (denoted here by their value).

As a very minor comment, for this example, and others in this section, you are generally eliding spaces (after : and ,) which makes reading the examples slightly harder.  I note in other places you do seem to include whitespace in the queries/results.

Regards,
Rob