Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update logical location kinds to accommodate XML and JSON paths #291

Closed
michaelcfanning opened this issue Nov 28, 2018 · 7 comments
Closed
Labels
2.1.0-CSD.1 Will be fixed in SARIF v2.1.0 CSD.1. e-ballot e-ballot-2 enhancement impact-non-breaking-change merged Changes merged into provisional draft. p2 Priority 2 issue to close resolved-fixed sdk-code-complete

Comments

@michaelcfanning
Copy link
Contributor

'kind' for results grouping, XML and JSON domains, and can we find a magical general term that absorbs function/member

@michaelcfanning michaelcfanning added the p2 Priority 2 issue to close label Jan 24, 2019
@michaelcfanning
Copy link
Contributor Author

Received some feedback from a team implementing a JSON-driven analysis

  • Some assistance with logical locations would be useful
  • In this scenario, the leaf node is not super interesting, the parent objects are what's interesting. Our design currently requires that a logicalIndex point into the detail object.
  • The scan team has two paths, one is qualified by actual array indices, e.g., object[5].property. They have another, more 'stable' form where they elide the actual index, e.g., object[].property. This data removal is analogous to eliding line location details (where document churn might invalidate them).

@ghost
Copy link

ghost commented Feb 25, 2019

EBALLOT PROPOSAL: add a set of logical location kinds to the open-ended logicalLocation.kind property value set that help users locate paths within JSON, XML or other mark-up documents.

API IMPACT: no literal API impact, as this is an open-ended string. Producers should favor one of our set of 'blessed' terms if they deem them relevant. Terms that will be referenced in the specification and in schema comments will now also include:

  • For JSON:
    • object
    • array
    • property
    • value
  • For XML:
    • element
    • text
    • attribute
    • comment
    • dtd
    • processingInstruction

@michaelcfanning michaelcfanning changed the title improve logicalLocation.kind Update logical location kinds to accommodate XML and JSON paths Feb 27, 2019
@ghost ghost added the to-be-written label Mar 1, 2019
@ghost ghost mentioned this issue Mar 1, 2019
ghost pushed a commit that referenced this issue Mar 4, 2019
ghost pushed a commit that referenced this issue Mar 4, 2019
@kupsch
Copy link

kupsch commented Mar 7, 2019

Have to state how to represent a dtd in the logical location as it appears that there is not an XPath representation for the DTD. Perhaps /!DOCTYPE

ghost pushed a commit that referenced this issue Mar 10, 2019
@ghost ghost self-assigned this Mar 14, 2019
@ghost ghost added enhancement 2.1.0-CSD.1 Will be fixed in SARIF v2.1.0 CSD.1. merged Changes merged into provisional draft. labels Mar 14, 2019
@kupsch
Copy link

kupsch commented Mar 18, 2019

To make it clear, it should be noted that the line number are not part of the text. The line number could also be stylized differently such as color.

@kupsch
Copy link

kupsch commented Mar 18, 2019

In the JSON example there are a couple of errors in the SARIF data:

results/0/locations/0/fullQualifiedLogicalName should be "/orders/0/productIds/1"
results/0/locations/0/logicalLocationIndex should be 3
results/1/locations/0/logicalLocationIndex should be 4

@ghost
Copy link

ghost commented Mar 18, 2019

@kupsch Thanks Jim. I noted those changes in this comment in #342, and will fix it up after the ballot ends.

ghost pushed a commit that referenced this issue Mar 19, 2019
This draft contains all the changes through "e-ballot #2," which
opened on Friday March 15 and will close on Friday March 22. It contains
changes for ballot issues #168, #291, #309, #320, #321, #326, #335,
and #341, as well as for previously approved issue #340.

It does _not_ contain changes for any issues from "e-ballot #3,"
which will open on Friday March 22 and close on Friday March 29.
@ghost
Copy link

ghost commented Mar 29, 2019

Approved in e-ballot #2.

@ghost ghost closed this as completed Mar 29, 2019
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.1.0-CSD.1 Will be fixed in SARIF v2.1.0 CSD.1. e-ballot e-ballot-2 enhancement impact-non-breaking-change merged Changes merged into provisional draft. p2 Priority 2 issue to close resolved-fixed sdk-code-complete
Projects
None yet
Development

No branches or pull requests

2 participants