Thank you! I will fix these.
At the same time I found a bug in the spec. It says `run.results` is required – must be present with `null` value if the tool failed to produce results (like, it crashed).
But it should allow `results` to be missing and default to `null`. Confirmed with Michael. I’m going to file an issue and make that change.
Larry
From: Yekaterina O'Neil <>
Sent: Monday, April 29, 2019 6:20 PM
To: Larry Golding (Myriad Consulting Inc) <>; Michael Fanning <>; OASIS SARIF TC Discussion List <>
Subject: RE: #403: Remove per-property "inheritance" from cached objects
th e readFlowLocations -> threadFlowLocations
Section 3.47.2, “If index is present, thisObject SHALL take
the all properties present on the cached object.” -> “If index is present, thisObject SHALL take all properties present on the cached object.”
Section 3.33.3, “of an logicalLocation object” -> “of a logicalLocation object”
k
From:
[ mailto: ] On Behalf Of Larry Golding (Myriad Consulting Inc)
Sent: Monday, April 29, 2019 8:57 AM
To: Michael Fanning < >; OASIS SARIF TC Discussion List <
>
Subject: [sarif] RE: #403: Remove per-property "inheritance" from cached objects
Michael,
Thanks for the explanation and the update.
I have restored the
index properties for address and logicalLocation . The change draft is:
https://github.com/oasis-tcs/sarif-spec/blob/master/Documents/ChangeDrafts/Accepted/sarif-v2.0-issue-403-v2-restore-address-logloc-index.docx
#403 is no longer schema-changing (although of course it is still breaking).
The provisional draft is up to date:
https://github.com/oasis-tcs/sarif-spec/blob/master/Documents/ProvisionalDrafts/sarif-v2.0-csd02-provisional.docx
I won’t bother to update the HTML draft until after the remaining two items I need to address this morning:
“Subsetting” the RFC5646 language spec as Michael proposed yesterday and Jim confirmed this morning.
Addressing Jim’s feedback on the address object redesign.
Thanks,
Larry
From: Michael Fanning <
>
Sent: Monday, April 29, 2019 7:34 AM
To: Larry Golding (Myriad Consulting Inc) < >; OASIS SARIF TC Discussion List <
>
Subject: RE: #403: Remove per-property "inheritance" from cached objects
Hello,
The problem here is that the properties merging feature we envisioned is, among other things, subject to a classic issue: additive changes are relatively straightforward but dealing with default values is not (particularly in a SARIF SDK _expression_ in a language such as C# or Java). To account for this, we would have needed to consider making all properties on cacheable types nullable, to allow for a ‘present but null’ signal to indicate something in the merge operation (such as ‘clear this value’).
And so we made the call to remove the property inheritance and to support object inheritance only. One correction: in order to minimize churn in the format, particularly as we’re in the end game when the TC can’t meet to discuss, we will retain the index properties for address and logicalLocation.
And so, the proposal can be simplified to: support object inheritance only for all cacheable types (and drop poorly thought through property level merging). These types are : thread flow locations, logical locations, addresses, web requests and web responses.
Thanks! After we merge the spec chance around the “language” property (thanks for the comment, Jim!), we will provide David with the doc for the eballot.
PLEASE NOTE: if we absolutely need to, we can identify a document change that’s required and reset the ballot. Every time we do this, we reset the ballot clock, and so of course we’re hoping to avoid it entirely. If you need to pull the alarm bell for some reason, please try to do it sooner than later. :)
Michael
From:
<
> On Behalf Of Larry Golding (Myriad Consulting Inc)
Sent: Sunday, April 28, 2019 4:29 PM
To: OASIS SARIF TC Discussion List <
>
Subject: [sarif] #403: Remove per-property "inheritance" from cached objects
Importance: High
Michael identified a problem with our mechanism for inheriting individual properties from cached objects. It breaks the SDK in ways that Michael will explain in a follow-on email .
I pushed and merged a change that fixes the problem. We can still reuse cached objects (so that, for example,
threadFlowLocations can be reused in multiple thread flows, satisfying Yekaterina’s scenario), but we do not inherit individual properties from the cached object:
https://github.com/oasis-tcs/sarif-spec/blob/master/Documents/ChangeDrafts/Accepted/sarif-v2.0-issue-403-no-inheritance.docx
As part of this change, we removed the
index properties from the address object and the logicalLocation object. These “hierarchical” objects have a parentIndex , and that , not
index , is what establishes a connection to the cache. These objects are very small (in contrast to threadFlowLocation , which can be huge if it includes a stack), so the benefit of caching the “leaf objects” is small.
As always, the provisional draft and its HTML version are up to date:
https://github.com/oasis-tcs/sarif-spec/blob/master/Documents/ProvisionalDrafts/sarif-v2.0-csd02-provisional.docx
https://github.com/oasis-tcs/sarif-spec/blob/master/Documents/ProvisionalDrafts/sarif-v2.0-csd02-provisional.htm
Please take a look. The plan is still to open the CSD 2 ballot on Monday.
This is it. We have no more open issues except the question Michael asked this morning about this choice of which standard to cite for the run.language property.
Thanks,
Larry
