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

--force message updates. handle 422 code from log post endpoint. #2656

Merged
merged 6 commits into from
Apr 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion ReleaseHistory.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,10 @@
* BRK: `fileRegionsCache` parameter is now required for the `InsertOptionalDataVisitor`. [#2642](https://github.com/microsoft/sarif-sdk/pull/2642)
* BRK: Add `IAnalysisLogger.TargetAnalysisComplete` method. [#2637](https://github.com/microsoft/sarif-sdk/pull/2637)
* BRK: Remove unused `quiet` parameter from `SarifLogger`. [#2639]https://github.com/microsoft/sarif-sdk/pull/2639
* BRK: Remove `CompuateHashData` and `AnalysisTargetToHashDataMap` properties from `SarifLogger` (in preference of new `fileRegionsCache` parameter. [#2639](https://github.com/microsoft/sarif-sdk/pull/2639)
* BRK: Remove `ComputeHashData` and `AnalysisTargetToHashDataMap` properties from `SarifLogger` (in preference of new `fileRegionsCache` parameter. [#2639](https://github.com/microsoft/sarif-sdk/pull/2639)
* BRK: Eliminate proactive hashing of artifacts in `SarifLogger` constructor when `OptionallyEmittedData.Hashes` is specified. [#2639](https://github.com/microsoft/sarif-sdk/pull/2639)
* BUG: Update user messages and code comments that refer to `--force` (replaced by `--log ForceOverwrite`). [#2656](https://github.com/microsoft/sarif-sdk/pull/2656)
* BUG: Handle return code 422 `UnprocessableEntity` when validating that log file POST endpoint is available. [#2656](https://github.com/microsoft/sarif-sdk/pull/2656)
* BUG: Eliminate erroneous `Posted log file successfully` message when context `PostUri` is non-null but empty. [#2655](https://github.com/microsoft/sarif-sdk/pull/2655)
* BUG: Resolves `IOException` raised by calling `FileSystem.ReadAllText` on file locked for write (but not read). [#2655](https://github.com/microsoft/sarif-sdk/pull/2655)
* BUG: Correct `toolComponent.language` regex in JSON schema. [#2653]https://github.com/microsoft/sarif-sdk/pull/2653
Expand Down
9 changes: 5 additions & 4 deletions src/Sarif.Driver/DriverExtensionMethods.cs
Original file line number Diff line number Diff line change
Expand Up @@ -130,14 +130,15 @@ private static void ReportInvalidOutputFormatOptions()
/// absent, because by default each transformed file is written to the path containing
/// corresponding input file.
///
/// However, similarly to the case of SingleFileOptionsBase, we _do_ want to set --force
/// whenever --inline is true, because there's no reason to force the user to type
/// "--force" when they've already said that they want to overwrite the input file
/// However, similarly to the case of SingleFileOptionsBase, we _do_ want to set
/// --log ForceOverwrite whenever --log Inline is true, because there's no reason
/// to force the user to type "--log ForceOverwrite;Inline" when they've said
/// that they want to overwrite the input file
/// (see https://github.com/microsoft/sarif-sdk/issues/1642).
///
/// So we introduce this method for three reasons:
/// 1) For symmetry with the SingleFileOptionsBase,
/// 2) To DRY out the logic for making --inline and --force consistent, and
/// 2) To DRY out the logic for making --log Inline and --log ForceOverwrite consistent, and
/// 3) To leave an obvious place to put output file option consistency logic if it's
/// needed in future.
/// </remarks>
Expand Down
6 changes: 3 additions & 3 deletions src/Sarif.Driver/DriverUtilities.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public static class DriverUtilities
/// A list of the paths to the output files.
/// </param>
/// <param name="force">
/// true if the --force option was specified.
/// true if the --log ForceOverwrite option was specified.
/// </param>
/// <param name="fileSystem">
/// An object that provides access to the file system.
Expand Down Expand Up @@ -49,7 +49,7 @@ public static bool ReportWhetherOutputFilesCanBeCreated(IEnumerable<string> outp
/// The path to the output file.
/// </param>
/// <param name="force">
/// true if the --force option was specified.
/// true if the --log ForceOverwrite option was specified.
/// </param>
/// <param name="fileSystem">
/// An object that provides access to the file system.
Expand Down Expand Up @@ -79,7 +79,7 @@ public static bool ReportWhetherOutputFileCanBeCreated(string outputFilePath, bo
/// The path to the output file.
/// </param>
/// <param name="force">
/// true if the --force option was specified.
/// true if the --log ForceOverwrite option was specified.
/// </param>
/// <param name="fileSystem">
/// An object that provides access to the file system.
Expand Down
11 changes: 8 additions & 3 deletions src/Sarif.Driver/Sdk/MultithreadedAnalyzeCommandBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using System.Diagnostics;
using System.IO;
using System.Linq;
using System.Net;
using System.Net.Http;
using System.Runtime.InteropServices;
using System.Threading.Channels;
Expand Down Expand Up @@ -286,6 +287,7 @@ public virtual TContext ValidateContext(TContext globalContext)
globalContext.PluginFilePaths,
shouldExist: required ? true : (bool?)null);


if (!string.IsNullOrEmpty(globalContext.PostUri))
{
try
Expand All @@ -294,9 +296,11 @@ public virtual TContext ValidateContext(TContext globalContext)
var content = new StringContent(string.Empty);
HttpResponseMessage httpResponseMessage = httpClient.PostAsync(globalContext.PostUri, content).GetAwaiter().GetResult();

// Internal server error means we found our server but it didn't like our malformed payload.
// That means we're all good! i.e., if we provide a good SARIF file we should succeed.
if (httpResponseMessage.StatusCode != System.Net.HttpStatusCode.InternalServerError)
// Internal server error means we found our server but it didn't like our malformed payload
// (in first implementation). In a server update, this condition returns 422 (unprocessable
// payload). We treat either return value as good, i.e. posting a valid SARIF file should work.
if (httpResponseMessage.StatusCode != HttpStatusCode.InternalServerError &&
httpResponseMessage.StatusCode != (HttpStatusCode)422)
{
globalContext.PostUri = null;
succeeded = false;
Expand All @@ -305,6 +309,7 @@ public virtual TContext ValidateContext(TContext globalContext)
catch (Exception e)
{
globalContext.PostUri = null;
succeeded = false;
globalContext.RuntimeErrors |= RuntimeConditions.ExceptionPostingLogFile;
globalContext.RuntimeExceptions ??= new List<Exception>();
globalContext.RuntimeExceptions.Add(e);
Expand Down
2 changes: 1 addition & 1 deletion src/Sarif/Errors.cs
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,7 @@ public static void LogFileAlreadyExists(IAnalysisContext context, string filePat

context.RuntimeErrors |= RuntimeConditions.FileAlreadyExists;

// The output file '{0}' already exists. Use --force to overwrite.
// The output file '{0}' already exists. Use --log ForceOverwrite to overwrite.
context.Logger.LogConfigurationNotification(
CreateNotification(
uri: null,
Expand Down
4 changes: 2 additions & 2 deletions src/Sarif/SdkResources.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion src/Sarif/SdkResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@
<value>All rules were explicitly disabled so there is no work to do.</value>
</data>
<data name="ERR997_FileAlreadyExists" xml:space="preserve">
<value>The output file '{0}' already exists. Use --force to overwrite.</value>
<value>The output file '{0}' already exists. Use --log ForceOverwrite to overwrite.</value>
</data>
<data name="PartioningVisitHappensAtSarifLogLevel" xml:space="preserve">
<value>Could not identify the log being partitioned. Call VisitSarifLog and provide the log to partition. This class is designed to create log files on a per-run basis (i.e., all partioned logs will contain a single run only).</value>
Expand Down
18 changes: 9 additions & 9 deletions src/Test.UnitTests.Sarif.Driver/DriverExtensionMethodsTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ private class ValidateSingleFileOutputOptionsTestCase
{
new ValidateSingleFileOutputOptionsTestCase
{
Title = "--inline and not --force",
Title = "--log Inline and not --log ForceOverwrite",
Options = new SingleFileOptionsBase
{
OutputFilePath = null,
Expand All @@ -46,7 +46,7 @@ private class ValidateSingleFileOutputOptionsTestCase

new ValidateSingleFileOutputOptionsTestCase
{
Title = "--inline and superfluous --force",
Title = "--log Inline and superfluous --log ForceOverwrite",
Options = new SingleFileOptionsBase
{
OutputFileOptions = new[] { FilePersistenceOptions.ForceOverwrite, FilePersistenceOptions.Inline },
Expand All @@ -58,7 +58,7 @@ private class ValidateSingleFileOutputOptionsTestCase

new ValidateSingleFileOutputOptionsTestCase
{
Title = "Output path with --force",
Title = "Output path with --log ForceOverwrite",
Options = new SingleFileOptionsBase
{
OutputFileOptions = new[] { FilePersistenceOptions.ForceOverwrite },
Expand All @@ -70,7 +70,7 @@ private class ValidateSingleFileOutputOptionsTestCase

new ValidateSingleFileOutputOptionsTestCase
{
Title = "Output path without --force",
Title = "Output path without --log ForceOverwrite",
Options = new SingleFileOptionsBase
{
OutputFileOptions = new[] { FilePersistenceOptions.None },
Expand All @@ -82,7 +82,7 @@ private class ValidateSingleFileOutputOptionsTestCase

new ValidateSingleFileOutputOptionsTestCase
{
Title = "Neither --inline nor output path",
Title = "Neither --log Inline nor output path",
Options = new SingleFileOptionsBase
{

Expand All @@ -94,7 +94,7 @@ private class ValidateSingleFileOutputOptionsTestCase

new ValidateSingleFileOutputOptionsTestCase
{
Title = "Both --inline and output path",
Title = "Both --log Inline and output path",
Options = new SingleFileOptionsBase
{
OutputFileOptions = new[] { FilePersistenceOptions.Inline },
Expand Down Expand Up @@ -141,7 +141,7 @@ private class ValidateMultipleFilesOutputOptionsTestCase
{
new ValidateMultipleFilesOutputOptionsTestCase
{
Title = "--force and not --inline",
Title = "--log ForceOverwrite and not --log Inline",
Options = new MultipleFilesOptionsBase
{
OutputFileOptions = new[] { FilePersistenceOptions.ForceOverwrite },
Expand All @@ -152,7 +152,7 @@ private class ValidateMultipleFilesOutputOptionsTestCase

new ValidateMultipleFilesOutputOptionsTestCase
{
Title = "--inline and not --force",
Title = "--log Inline and not --log ForceOverwrite",
Options = new MultipleFilesOptionsBase
{
OutputFileOptions = new[] { FilePersistenceOptions.Inline },
Expand All @@ -163,7 +163,7 @@ private class ValidateMultipleFilesOutputOptionsTestCase

new ValidateMultipleFilesOutputOptionsTestCase
{
Title = "--inline and superfluous --force",
Title = "--log Inline and superfluous --log ForceOverwrite",
Options = new MultipleFilesOptionsBase
{
OutputFileOptions = new[] { FilePersistenceOptions.ForceOverwrite, FilePersistenceOptions.Inline },
Expand Down