OASIS Static Analysis Results Interchange Format (SARIF) TC

 View Only
Expand all | Collapse all

RE: #403: Remove per-property "inheritance" from cached objects

  • 1.  RE: #403: Remove per-property "inheritance" from cached objects

    Posted 04-29-2019 15:57
    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 <Michael.Fanning@microsoft.com> Sent: Monday, April 29, 2019 7:34 AM To: Larry Golding (Myriad Consulting Inc) <v-lgold@microsoft.com>; OASIS SARIF TC Discussion List <sarif@lists.oasis-open.org> 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: sarif@lists.oasis-open.org < sarif@lists.oasis-open.org > On Behalf Of Larry Golding (Myriad Consulting Inc) Sent: Sunday, April 28, 2019 4:29 PM To: OASIS SARIF TC Discussion List < sarif@lists.oasis-open.org > 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  


  • 2.  RE: #403: Remove per-property "inheritance" from cached objects

    Posted 04-30-2019 01:30
    -           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: sarif@lists.oasis-open.org [mailto:sarif@lists.oasis-open.org] On Behalf Of Larry Golding (Myriad Consulting Inc) Sent: Monday, April 29, 2019 8:57 AM To: Michael Fanning <Michael.Fanning@microsoft.com>; OASIS SARIF TC Discussion List <sarif@lists.oasis-open.org> 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 < Michael.Fanning@microsoft.com > Sent: Monday, April 29, 2019 7:34 AM To: Larry Golding (Myriad Consulting Inc) < v-lgold@microsoft.com >; OASIS SARIF TC Discussion List < sarif@lists.oasis-open.org > 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: sarif@lists.oasis-open.org < sarif@lists.oasis-open.org > On Behalf Of Larry Golding (Myriad Consulting Inc) Sent: Sunday, April 28, 2019 4:29 PM To: OASIS SARIF TC Discussion List < sarif@lists.oasis-open.org > 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