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

Suggesting file path display base to viewers #396

Closed
ghost opened this issue Apr 25, 2019 · 2 comments
Closed

Suggesting file path display base to viewers #396

ghost opened this issue Apr 25, 2019 · 2 comments
Labels
design-approved The TC approved the design and I can write the change draft enhancement impact-non-breaking-change merged Changes merged into provisional draft. resolved-fixed

Comments

@ghost
Copy link

ghost commented Apr 25, 2019

From @kupsch:

Jim: In accordance with @michaelcfanning's feedback not to give "SHOULD" guidance to viewers, I weakened it to MAY when I transcribed your proposal here.

NOTE: This was the original proposal. See the REVISED PROPOSAL below.

PROPOSAL

Specify a distinguished property name such as DISPLAYBASE in run.originalUriBaseIds that guides viewers in displaying path names. If the value is set, consumers MAY interpret it as a hint to display URIs relative to to the DISPLAYBASE uriBaseId. Checking if the path is contained in DISPLAYBASE should be done after resolving and normalizing both the DISPLAYBASE uri and the uri being displayed using the algorithm in §3.14.14. If the displayed URI's scheme, authority and initial path segments match DISPLAYBASE, only display the remaining path segments (or '.' if there are no remaining segments). If only the scheme and authority match, display the entire absolute path. Otherwise, display the URI as an absolute URI (or in some other appropriate form such as a <uriBaseId, uri> pair).

Producers SHOULD NOT use DISPLAYBASE as a uriBaseId in artifactLocations to enable a result management system, or translator to set or change the value after the fact without having to change all uses of DISPLAYBASE.

We could reserve all uriBaseIds that start with SARIF_ for future use, and use SARIF_DISPLAYBASE for the name. That allows us to define other distinguished uriBaseId values in future without conflicting with producer uses.

EXAMPLE

Given the following originalUriBaseIds:

"originalUriBaseIds":  {
   "WEBHOST":  {
     "uri": "http://www.example.com"
   },
   "ROOT":  {
     "uri": "file:///"
   },
   "HOME":  {
     "uriBaseId": "ROOT",
     "uri": "/home/user/"
   },
   "PACKAGE":  {
     "uriBaseId":  "HOME",
     "uri": "mySoftware/"
   },
   "SRC":  {
     "uriBaseId": "PACKAGE",
     "uri": "src/"
   },
   "DISPLAYBASE":  {
     "uriBaseId": "PACKAGE"
   }
}

All these equivalent locations below would display as src/f.c because the scheme, authority, and path segments prefix match:

{
   "uriBaseId": "SRC",
   "uri": "f.c"
}
{
   "uriBaseId": "PACKAGE",
   "uri": "src/f.c"
}
{
  "uri": "file:///home/user/mySoftware/src/f.c"
}

All these equivalent locations below would display as /usr/include/stdio.h because the scheme and authority match, but not the path:

{
   "uriBaseId": "ROOT",
   "uri": "/usr/include/stdio.h"
}
{
   "uriBaseId": "SRC",
   "uri": "/usr/include/stdio.h"
}
{
   "uri": "file:///usr/include/stdio.h"
}

All these equivalent locations below would display as http://www.example.com/hello because the scheme and authority do not match:

{
  "uriBaseId": "WEBHOST",
  "uri": "hello"
}
{
   "uri": "http://www.example.com/hello"
}

If DISPLAYBASE was changed to

   {
     "uriBaseId": "HOME"
   }

and no other changes were made, the only change to the above display URIs would be src/f.c would be displayed as mySoftware/src/f.c. All other display values would be unchanged.

@ghost ghost self-assigned this Apr 25, 2019
@ghost ghost added design-approved The TC approved the design and I can write the change draft schema-todo and removed discussion-ongoing labels Apr 25, 2019
@ghost
Copy link
Author

ghost commented Apr 27, 2019

REVISED PROPOSAL

After Jim presented this proposal, there was a lot of follow-on discussion with @kupsch, @michaelcfanning, and @lgolding. All agreed that it is valuable to provide this hint to viewers; the discussion was about how to represent it:

  1. As a reserved property name DISPLAYBASE in run.originalUriBaseIds.
  2. As a property name SARIF_DISPLAYBASE in a reserved SARIF_-prefixed "namespace" in run.originalUriBaseIds.
  3. As a new property run.displayBase.

All of these had drawbacks:

  • #1 requires a reserved name, and requires that name to be treated specially (not actually used as a uriBaseId value anywhere else in the log file). Also, it implies that if we added any other "special" values in the future, it would be a breaking change, since an implementer might have already used whatever value we chose.
  • #2 also requires a reserved name that can't actually be used as a uriBaseId elsewhere in the log file. It has the advantage over #1 of allowing additional special values to be defined in a non-breaking way, at the cost of defining a mini-language on the property names.
  • #3 doesn't have the drawbacks of #1 or #2, but it requires a schema change, and if we define other special URIs in future, we'd have a proliferation of properties on the run object.

@michaelcfanning and I propose this alternative:

  1. Define a property run.specialLocations with a single property displayBase whose value is an artifactLocation. In future, additional specially treated locations can be defined in a non-breaking way by defining new properties on run.specialLocations.

It has these advantages:

  • It doesn't require a reserved and specially treated uriBaseId value.
  • It doesn't require a namespace on uriBaseId values.
  • It allows new "special" locations to be defined in a non-breaking way.
  • It avoids a proliferation of properties on the run object.

It has this disadvantage:

  • It requires a schema change to define the specialLocations object and add the property run.specialLocations.

@michaelcfanning and I are willing to take the pain of the schema change to get the other benefits.

@ghost ghost added merged Changes merged into provisional draft. resolved-fixed labels Apr 27, 2019
@ghost ghost closed this as completed Apr 27, 2019
ghost pushed a commit that referenced this issue Apr 27, 2019
@ghost ghost removed the schema-todo label Apr 29, 2019
@ghost
Copy link
Author

ghost commented Apr 29, 2019

I confirm that this change is correctly in the schema.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design-approved The TC approved the design and I can write the change draft enhancement impact-non-breaking-change merged Changes merged into provisional draft. resolved-fixed
Projects
None yet
Development

No branches or pull requests

0 participants