@kupsch feedback responses

[ghost]

This is @kupsch's raw response to the changes I made pursuant to #366 in the draft https://github.com/oasis-tcs/sarif-spec/blob/master/Documents/ChangeDrafts/Accepted/sarif-v2.0-issues-366-367-369-370.docx.

I'll address these inline, and break any substantial items into their own issues.

@michaelcfanning FYI

---

p.16 1.2 Terminology - artifact definition: Although a database can be accessed via a URL, the URL is really an artifact on the web server that happens to map to a database table, so only indirectly is it a database table.

Response: I changed it to "database table accessed via an HTTP request"

---

p.36 3.10.1 General

"location of the file at the time" -> "location of the artifact at the time"

"For additional normalization requirements for URIs that use the "file"
scheme ..." -> "file scheme URI normalization SHALL use a modified version of RFC 3986 described in the next section ..."

Response: Done

---

p.37 3.10.2 URIs that use the file scheme

Change section name to "Normalizing file scheme URIs"

Response: Done

Add after list item 2: "Remove empty path segments"

Response: Done

---

p.39 3.11.3 Plain text messages

"Lline" -> "Line"

Response: Done

---

p.181 Appendix D (Normative) Production of SARIF by converters

"version [SEMVER]" -> "version [SEMVER]"

Response: Done (I had to stare at this to understand that you had detected an extra space!)

---

p.181 Appendix D (Normative) Production of SARIF by converters

The clause "but SHOULD NOT attempt to populate result.analysisTarget"
should be removed, as for many tools the analysisTarget is known

Response: The spec is correct as it stands. It only tells the converter not to populate analysisTarget when the converter doesn't know the analysis target:

• A converter that knows which artifact a result was detected in, but not which artifact the analysis tool was originally instructed to scan [emphasis added], SHOULD populate result.locations (§3.26.12), but SHOULD NOT attempt to populate result.analysisTarget (§3.26.13).

==================================================

A couple of other clarification that I think should be made

---

p.29 3.4.5 index property

There should be a mechanism to redact multiple baseUriIds, but still allow them to be different directories in the file system. If the redactionToken is used then there needs to be a note that if the redactionToken is present in segment, then each use within a uri should be considered a unique (undisclosed) location in the file system, just like a uri with '..'. If the redactionToken appears in a baseUriId's uri then the baseUriId should be treated as if it were a unique
(undisclosed) directory location, but all uses of the that baseUriId with the redacted segment should be considered to have the same unique
(undisclosed) path prefix. For instance if the redactionToken is "[REDACTED]", and a baseUriId named X has a uri of "[REDACTED]", then {baseUriId="X", uri="f.c"} and {baseUriId="X", uri"f.c"} are the same path, whereas {uri="[REDACTED]/f.c} and {uri="[REDACTED]/f.c} should not be considered the same path.

An alternative would be to allow an index of -1 to indicate redaction.

Response: I filed #377, "Each redaction token in an originalUriBaseId represents a unique location."

---

p.147 response object

The response object should have a mechanism to indicate that no response was received from the server. This could happen due to a DNS failure or a connectivity issue such as the server not running or the host or port being inaccessible. Should add a requestFailed and requestFailureReason at a minimum. This is similar to an invocation where to process creation failed.

Response: I filed #378, "Add response properties to describe failed requests"

[ghost]

I fixed the small items (except one where the spec was correct as it stands) and opened #377 and #378 for the substantive ones