Merge pull request 8117 from hotfix/v4.4.10 into master

51Degrees · Jan 31, 2023 · 0000129 · 0000129
2 parents 10d9eab + e29d4e7
commit 0000129
Show file tree

Hide file tree

Showing 27 changed files with 136 additions and 35 deletions.
diff --git a/FiftyOne.Pipeline.CloudRequestEngine/FlowElements/CloudRequestEngine.cs b/FiftyOne.Pipeline.CloudRequestEngine/FlowElements/CloudRequestEngine.cs
@@ -281,6 +281,16 @@ private string ProcessResponse(
                     var errors = jObj.Value<JArray>("errors");
                     messages.AddRange(errors.Children<JValue>().Select(t => t.Value.ToString()));
                 }
+
+                // Log any warnings that were returned.
+                if (jObj.ContainsKey("warnings"))
+                {
+                    var warnings = jObj.Value<JArray>("warnings");
+                    foreach (var warning in warnings.Children<JValue>().Select(t => t.Value.ToString()))
+                    {
+                        Logger.LogWarning(warning);
+                    }
+                }
             }
 
             // If there were no errors but there was also no other data

diff --git a/...ements/FiftyOne.Pipeline.JavaScriptBuilderElement/FlowElement/JavaScriptBuilderElement.cs b/...ements/FiftyOne.Pipeline.JavaScriptBuilderElement/FlowElement/JavaScriptBuilderElement.cs
@@ -42,6 +42,7 @@
 using FiftyOne.Pipeline.Engines.Data;
 using FiftyOne.Pipeline.Engines;
 using Newtonsoft.Json;
+using System.Globalization;
 
 namespace FiftyOne.Pipeline.JavaScriptBuilder.FlowElement
 {
@@ -143,6 +144,11 @@ public class JavaScriptBuilderElement :
 		public override IList<IElementPropertyMetaData> Properties => 
 			_properties;
 
+        /// <summary>
+        /// Flag to record whether we have already logged an error about an invalid uri.
+        /// This prevents us from spamming the user with error messages.
+        /// </summary>
+        private bool _invalidUriLogged = false;
 
         /// <summary>
         /// Default constructor.
@@ -357,7 +363,20 @@ private void SetUp(IFlowData data)
                     endpoint = endpoint.Substring(1);
                 }
 
-                url = new Uri($"{protocol}://{host}{endpoint}");
+                var urlString = $"{protocol}://{host}{endpoint}";
+                try
+                {
+                    url = new Uri(urlString);
+                }
+                catch (UriFormatException ex)
+                {
+                    if (_invalidUriLogged == false) 
+                    {
+                        _invalidUriLogged = true;
+                        Logger.LogError(ex, string.Format(CultureInfo.InvariantCulture,
+                            Messages.ExceptionUriInvalid, urlString));
+                    }
+                }
             }
 
             // With the gathered resources, build a new JavaScriptResource.

diff --git a/FiftyOne.Pipeline.Elements/FiftyOne.Pipeline.JavaScriptBuilderElement/Messages.Designer.cs b/FiftyOne.Pipeline.Elements/FiftyOne.Pipeline.JavaScriptBuilderElement/Messages.Designer.cs
diff --git a/FiftyOne.Pipeline.Elements/FiftyOne.Pipeline.JavaScriptBuilderElement/Messages.resx b/FiftyOne.Pipeline.Elements/FiftyOne.Pipeline.JavaScriptBuilderElement/Messages.resx
@@ -126,4 +126,7 @@
   <data name="ExceptionObjectNameInvalid" xml:space="preserve">
     <value>JavaScriptBuilder ObjectName '{0}' is invalid. This must be a valid JavaScript type identifier.</value>
   </data>
+  <data name="ExceptionUriInvalid" xml:space="preserve">
+    <value>The generated Uri '{0}' is invalid. Please check your JavaScriptBuilderElement protocol, host and endpoint configuration.</value>
+  </data>
 </root>
diff --git a/...Elements/FiftyOne.Pipeline.JavaScriptBuilderElement/Templates/JavaScriptResource.mustache b/...Elements/FiftyOne.Pipeline.JavaScriptBuilderElement/Templates/JavaScriptResource.mustache
@@ -12,6 +12,11 @@
         console.log(json.error);
     }
 
+    // Log any warnings returned in the JSON object.
+    if (json.warnings !== undefined) {
+        console.log(json.warnings);
+    }
+
     // Set to true when the JSON object is complete.
     var completed = false;
 

diff --git a/...line.JavaScriptBuilderElementTests/FiftyOne.Pipeline.JavaScriptBuilderElementTests.csproj b/...line.JavaScriptBuilderElementTests/FiftyOne.Pipeline.JavaScriptBuilderElementTests.csproj
@@ -9,7 +9,7 @@
   </PropertyGroup>
 
   <ItemGroup>
-    <PackageReference Include="FiftyOne.Common.TestHelpers" Version="4.4.1" />
+    <PackageReference Include="FiftyOne.Common.TestHelpers" Version="4.4.3" />
     <PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.5.0" />
     <PackageReference Include="Moq" Version="4.13.1" />
     <PackageReference Include="MSTest.TestAdapter" Version="2.1.0" />

diff --git a/....Pipeline.Elements/FiftyOne.Pipeline.JsonBuilderElement/FlowElement/JsonBuilderElement.cs b/....Pipeline.Elements/FiftyOne.Pipeline.JsonBuilderElement/FlowElement/JsonBuilderElement.cs
@@ -372,7 +372,7 @@ private void AddJavaScriptProperties(IFlowData data,
         /// <exception cref="ArgumentNullException">
         /// Thrown if one of the supplied parameters is null
         /// </exception>
-        protected static void AddErrors(IFlowData data,
+        protected virtual void AddErrors(IFlowData data,
             Dictionary<string, object> allProperties)
         {
             if (data == null) throw new ArgumentNullException(nameof(data));

diff --git a/FiftyOne.Pipeline.Engines/Constants.cs b/FiftyOne.Pipeline.Engines/Constants.cs
@@ -59,5 +59,10 @@ public static class Constants
         /// update distribution endpoint at exactly the same time.
         /// </summary>
         public const int DATA_UPDATE_RANDOMISATION_DEFAULT = 10 * 60;
+
+        /// <summary>
+        /// The default endpoint for the JavaScript to call when requesting json data
+        /// </summary>
+        public const string DEFAULT_JSON_ENDPOINT = "/51dpipeline/json";
     }
 }
diff --git a/FiftyOne.Pipeline.Engines/FiftyOne.Pipeline.Engines.csproj b/FiftyOne.Pipeline.Engines/FiftyOne.Pipeline.Engines.csproj
@@ -38,7 +38,7 @@
 
   <ItemGroup>
     <PackageReference Include="FiftyOne.Caching" Version="4.4.1" />
-    <PackageReference Include="FiftyOne.Common" Version="4.4.1" />
+    <PackageReference Include="FiftyOne.Common" Version="4.4.3" />
     <PackageReference Include="Microsoft.CodeAnalysis.FxCopAnalyzers" Version="2.9.8">
       <PrivateAssets>all</PrivateAssets>
       <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>

diff --git a/FiftyOne.Pipeline.Web.Shared/Messages.Designer.cs b/FiftyOne.Pipeline.Web.Shared/Messages.Designer.cs
diff --git a/FiftyOne.Pipeline.Web.Shared/Messages.resx b/FiftyOne.Pipeline.Web.Shared/Messages.resx
@@ -129,6 +129,9 @@
   <data name="MessageEvidenceError" xml:space="preserve">
     <value>Problem retrieving evidence from request</value>
   </data>
+  <data name="MessageInvalidForm" xml:space="preserve">
+    <value>The form data POSTed was invalid, so will not be added to the evidence.</value>
+  </data>
   <data name="MessageJavaScriptCachingError" xml:space="preserve">
     <value>Problem setting response headers for 51Degrees.core.js</value>
   </data>

diff --git a/...tyOne.Pipeline.CloudRequestEngine.Tests/FiftyOne.Pipeline.CloudRequestEngine.Tests.csproj b/...tyOne.Pipeline.CloudRequestEngine.Tests/FiftyOne.Pipeline.CloudRequestEngine.Tests.csproj
@@ -7,7 +7,7 @@
   </PropertyGroup>
 
   <ItemGroup>
-    <PackageReference Include="FiftyOne.Common.TestHelpers" Version="4.4.1" />
+    <PackageReference Include="FiftyOne.Common.TestHelpers" Version="4.4.3" />
     <PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.5.0" />
     <PackageReference Include="Moq" Version="4.13.1" />
     <PackageReference Include="MSTest.TestAdapter" Version="2.1.0" />

diff --git a/.../FiftyOne.Pipeline.Engines.FiftyOne.Tests/FiftyOne.Pipeline.Engines.FiftyOne.Tests.csproj b/.../FiftyOne.Pipeline.Engines.FiftyOne.Tests/FiftyOne.Pipeline.Engines.FiftyOne.Tests.csproj
@@ -9,7 +9,7 @@
   </PropertyGroup>
 
   <ItemGroup>
-    <PackageReference Include="FiftyOne.Common.TestHelpers" Version="4.4.1" />
+    <PackageReference Include="FiftyOne.Common.TestHelpers" Version="4.4.3" />
     <PackageReference Include="Microsoft.Extensions.DependencyInjection" Version="2.1.1" />
     <PackageReference Include="Microsoft.Extensions.Logging.Console" Version="2.1.1" />
     <PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.5.0" />

diff --git a/Tests/FiftyOne.Pipeline.Engines.Tests/FiftyOne.Pipeline.Engines.Tests.csproj b/Tests/FiftyOne.Pipeline.Engines.Tests/FiftyOne.Pipeline.Engines.Tests.csproj
@@ -19,7 +19,7 @@
   </ItemGroup>
 
   <ItemGroup>
-    <PackageReference Include="FiftyOne.Common.TestHelpers" Version="4.4.1" />
+    <PackageReference Include="FiftyOne.Common.TestHelpers" Version="4.4.3" />
     <PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.5.0" />
     <PackageReference Include="Moq" Version="4.13.1" />
     <PackageReference Include="MSTest.TestAdapter" Version="2.1.0" />

diff --git a/Tests/FiftyOne.Pipeline.Engines.Tests/Performance/PipelineOverheadTests.cs b/Tests/FiftyOne.Pipeline.Engines.Tests/Performance/PipelineOverheadTests.cs
@@ -88,7 +88,7 @@ public void PipelineOverhead_NoCache()
             stopwatch.Stop();
 
             double msOverheadPerCall =
-                stopwatch.ElapsedMilliseconds / iterations;
+                stopwatch.ElapsedMilliseconds / (double)iterations;
             Assert.IsTrue(msOverheadPerCall < 0.1,
                 $"Pipeline overhead per Process call was " +
                 $"{msOverheadPerCall}ms. Maximum permitted is 0.1ms");
@@ -124,7 +124,7 @@ public void PipelineOverhead_Cache()
             var end = DateTime.UtcNow;
 
             double msOverheadPerCall =
-                end.Subtract(start).TotalMilliseconds / iterations;
+                end.Subtract(start).TotalMilliseconds / (double)iterations;
             Assert.IsTrue(msOverheadPerCall < 0.1,
                 $"Pipeline overhead per Process call was " +
                 $"{msOverheadPerCall}ms. Maximum permitted is 0.1ms");
@@ -170,8 +170,7 @@ public void PipelineOverhead_Concurrency()
 
             // Calculate the time per call from the task results.
             List<double> results = tasks
-                .Select(t => t.Result.TotalMilliseconds / iterations).ToList();
-
+                .Select(t => t.Result.TotalMilliseconds / (double)iterations).ToList();
             Assert.IsTrue(results.All(r => r < 0.1),
                     $"Pipeline overhead per Process call was too high for " +
                     $"{results.Count(r => r > 0.1)} out of {threads} threads." +

diff --git a/Web Integration/Examples/ExampleFrameworkWebsite/AspNet Framework MVC Example.csproj b/Web Integration/Examples/ExampleFrameworkWebsite/AspNet Framework MVC Example.csproj
@@ -1,4 +1,4 @@
-<?xml version="1.0" encoding="utf-8"?>
+<?xml version="1.0" encoding="utf-8"?>
 <Project ToolsVersion="15.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
   <Import Project="..\..\..\packages\Microsoft.CodeDom.Providers.DotNetCompilerPlatform.2.0.0\build\net46\Microsoft.CodeDom.Providers.DotNetCompilerPlatform.props" Condition="Exists('..\..\..\packages\Microsoft.CodeDom.Providers.DotNetCompilerPlatform.2.0.0\build\net46\Microsoft.CodeDom.Providers.DotNetCompilerPlatform.props')" />
   <Import Project="$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props" Condition="Exists('$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props')" />
@@ -50,7 +50,7 @@
       <HintPath>..\..\..\packages\FiftyOne.Caching.4.4.1\lib\netstandard2.0\FiftyOne.Caching.dll</HintPath>
     </Reference>
     <Reference Include="FiftyOne.Common, Version=4.4.1.0, Culture=neutral, processorArchitecture=MSIL">
-      <HintPath>..\..\..\packages\FiftyOne.Common.4.4.1\lib\netstandard2.0\FiftyOne.Common.dll</HintPath>
+      <HintPath>..\..\..\packages\FiftyOne.Common.4.4.3\lib\netstandard2.0\FiftyOne.Common.dll</HintPath>
     </Reference>
     <Reference Include="Microsoft.CSharp" />
     <Reference Include="Newtonsoft.Json, Version=13.0.0.0, Culture=neutral, PublicKeyToken=30ad4fe6b2a6aeed, processorArchitecture=MSIL">

diff --git a/Web Integration/Examples/ExampleFrameworkWebsite/packages.config b/Web Integration/Examples/ExampleFrameworkWebsite/packages.config
@@ -1,9 +1,9 @@
-<?xml version="1.0" encoding="utf-8"?>
+<?xml version="1.0" encoding="utf-8"?>
 <packages>
   <package id="Antlr" version="3.5.0.2" targetFramework="net472" />
   <package id="bootstrap" version="3.4.1" targetFramework="net472" />
   <package id="FiftyOne.Caching" version="4.4.1" targetFramework="net462" />
-  <package id="FiftyOne.Common" version="4.4.1" targetFramework="net462" />
+  <package id="FiftyOne.Common" version="4.4.3" targetFramework="net462" />
   <package id="jQuery" version="3.3.1" targetFramework="net472" />
   <package id="jQuery.Validation" version="1.17.0" targetFramework="net472" />
   <package id="Microsoft.AspNet.Mvc" version="5.2.4" targetFramework="net472" />

diff --git a/Web Integration/FiftyOne.Pipeline.Web.Framework/PipelineModule.cs b/Web Integration/FiftyOne.Pipeline.Web.Framework/PipelineModule.cs
@@ -83,7 +83,7 @@ private void OnBeginRequestJavascript(object sender, EventArgs e)
                     FiftyOneJsProvider.GetInstance().ServeJavascript(context);
                     HttpContext.Current.ApplicationInstance.CompleteRequest();
                 }
-                if (context.Request.Path.EndsWith("51dpipeline/json",
+                if (context.Request.Path.EndsWith(Engines.Constants.DEFAULT_JSON_ENDPOINT,
                       StringComparison.OrdinalIgnoreCase))
                 {
                     FiftyOneJsProvider.GetInstance().ServeJson(context);
@@ -99,7 +99,10 @@ private void OnEndRequest(object sender, EventArgs e)
             if (context != null)
             {
                 PipelineCapabilities caps = context.Request.Browser as PipelineCapabilities;
-                caps.FlowData.Dispose();
+                if (caps != null)
+                {
+                    caps.FlowData.Dispose();
+                }
             }
         }
 

diff --git a/Web Integration/FiftyOne.Pipeline.Web.Framework/Providers/CapabilitiesProvider.cs b/Web Integration/FiftyOne.Pipeline.Web.Framework/Providers/CapabilitiesProvider.cs
@@ -68,8 +68,22 @@ public CapabilitiesProvider(HttpCapabilitiesDefaultProvider parent) : base(paren
         public override HttpBrowserCapabilities GetBrowserCapabilities(
             HttpRequest request)
         {
+            if (request == null) { throw new ArgumentNullException(nameof(request)); }
+
             HttpBrowserCapabilities caps;
             var baseCaps = base.GetBrowserCapabilities(request);
+
+            if(request.RequestContext.HttpContext.Response.HeadersWritten == true)
+            {
+                // The response has already been sent so just use the base capabilities.
+                // This can occur when using packages such as OWIN SignalR.
+                // SignalR intercepts certain requests, writes a response and closes it.
+                // This triggers our PipelineModule.OnEndRequest handler, which tries to
+                // get the browser capabilities object.
+                // Since it doesn't exist yet, it creates it and ends up doing all the
+                // processing for no reason at all.
+                return baseCaps;
+            }            
 
             var flowData = WebPipeline.Process(request);
             if (flowData != null)

diff --git a/Web Integration/FiftyOne.Pipeline.Web.Framework/Providers/SetHeadersProvider.cs b/Web Integration/FiftyOne.Pipeline.Web.Framework/Providers/SetHeadersProvider.cs
@@ -122,15 +122,18 @@ public static void SetHeaders(HttpContext context,
             if (context == null) throw new ArgumentNullException(nameof(context));
             if (headersToSet == null) throw new ArgumentNullException(nameof(headersToSet));
 
-            foreach (var header in headersToSet)
+            if (context.Response.HeadersWritten == false)
             {
-                if (context.Response.Headers.AllKeys.Contains(header.Key))
+                foreach (var header in headersToSet)
                 {
-                    context.Response.Headers[header.Key] += $",{header.Value}";
-                }
-                else
-                {
-                    context.Response.Headers.Add(header.Key, header.Value);
+                    if (context.Response.Headers.AllKeys.Contains(header.Key))
+                    {
+                        context.Response.Headers[header.Key] += $",{header.Value}";
+                    }
+                    else
+                    {
+                        context.Response.Headers.Add(header.Key, header.Value);
+                    }
                 }
             }
         }

diff --git a/Web Integration/FiftyOne.Pipeline.Web.Framework/WebPipeline.cs b/Web Integration/FiftyOne.Pipeline.Web.Framework/WebPipeline.cs
@@ -176,7 +176,7 @@ private WebPipeline()
                         BuilderName = nameof(JavaScriptBuilderElement),
                         BuildParameters = new Dictionary<string, object>()
                         {
-                            { "EndPoint", "/51dpipeline/json" }
+                            { "EndPoint", Engines.Constants.DEFAULT_JSON_ENDPOINT }
                         }
                     });
                 }
@@ -186,7 +186,7 @@ private WebPipeline()
                     // the endpoint is specified. If not, add it.
                     if (javascriptConfig.Single().BuildParameters.ContainsKey("EndPoint") == false)
                     {
-                        javascriptConfig.Single().BuildParameters.Add("EndPoint", "/51dpipeline/json");
+                        javascriptConfig.Single().BuildParameters.Add("EndPoint", Engines.Constants.DEFAULT_JSON_ENDPOINT);
                     }
                 }
             }

diff --git a/Web Integration/FiftyOne.Pipeline.Web/FiftyOneStartup.cs b/Web Integration/FiftyOne.Pipeline.Web/FiftyOneStartup.cs
@@ -232,7 +232,7 @@ private static void AddJsElements(PipelineOptions options)
                     BuilderName = nameof(JavaScriptBuilderElement),
                     BuildParameters = new Dictionary<string, object>()
                     {
-                        { "EndPoint", "/51dpipeline/json" }
+                        { "EndPoint", Engines.Constants.DEFAULT_JSON_ENDPOINT }
                     }
                 });
             }
@@ -242,7 +242,7 @@ private static void AddJsElements(PipelineOptions options)
                 // the endpoint is specified. If not, add it.
                 if (javascriptConfig.Single().BuildParameters.ContainsKey("EndPoint") == false)
                 {
-                    javascriptConfig.Single().BuildParameters.Add("EndPoint", "/51dpipeline/json");
+                    javascriptConfig.Single().BuildParameters.Add("EndPoint", Engines.Constants.DEFAULT_JSON_ENDPOINT);
                 }
             }
         }

diff --git a/Web Integration/FiftyOne.Pipeline.Web/Services/FiftyOneJSService.cs b/Web Integration/FiftyOne.Pipeline.Web/Services/FiftyOneJSService.cs
@@ -129,7 +129,7 @@ public bool ServeJson(HttpContext context)
             if (context == null) throw new ArgumentNullException(nameof(context));
 
             bool result = false;
-            if (context.Request.Path.Value.EndsWith("51dpipeline/json",
+            if (context.Request.Path.Value.EndsWith(Engines.Constants.DEFAULT_JSON_ENDPOINT,
                 StringComparison.OrdinalIgnoreCase))
             {
                 ServeCoreJson(context);

diff --git a/Web Integration/FiftyOne.Pipeline.Web/Services/WebRequestEvidenceService.cs b/Web Integration/FiftyOne.Pipeline.Web/Services/WebRequestEvidenceService.cs
@@ -22,6 +22,7 @@
 
 using System;
 using System.Globalization;
+using System.IO;
 using System.Linq;
 using FiftyOne.Pipeline.Core.Data;
 using FiftyOne.Pipeline.Web.Shared;
@@ -148,11 +149,19 @@ public void AddEvidenceFromRequest(IFlowData flowData, HttpRequest httpRequest)
                 if (httpRequest.Method == Shared.Constants.METHOD_POST &&
                     Shared.Constants.CONTENT_TYPE_FORM.Contains(httpRequest.ContentType))
                 {
-                    foreach (var formValue in httpRequest.Form)
+                    try
                     {
-                        string evidenceKey = Core.Constants.EVIDENCE_QUERY_PREFIX +
-                            Core.Constants.EVIDENCE_SEPERATOR + formValue.Key;
-                        CheckAndAdd(flowData, evidenceKey, formValue.Value.ToString());
+                        foreach (var formValue in httpRequest.Form)
+                        {
+                            string evidenceKey = Core.Constants.EVIDENCE_QUERY_PREFIX +
+                                Core.Constants.EVIDENCE_SEPERATOR + formValue.Key;
+                            CheckAndAdd(flowData, evidenceKey, formValue.Value.ToString());
+                        }
+                    }
+                    catch (InvalidDataException e)
+                    {
+                        _logger.LogInformation(e,
+                            Messages.MessageInvalidForm);
                     }
                 }
                 if (GetSessionEnabled(httpRequest))

diff --git a/...ration/Tests/FiftyOne.Pipeline.Web.Shared.Tests/FiftyOne.Pipeline.Web.Shared.Tests.csproj b/...ration/Tests/FiftyOne.Pipeline.Web.Shared.Tests/FiftyOne.Pipeline.Web.Shared.Tests.csproj
@@ -7,7 +7,7 @@
   </PropertyGroup>
 
   <ItemGroup>
-    <PackageReference Include="FiftyOne.Common.TestHelpers" Version="4.4.1" />
+    <PackageReference Include="FiftyOne.Common.TestHelpers" Version="4.4.3" />
     <PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.2.0" />
     <PackageReference Include="Moq" Version="4.13.1" />
     <PackageReference Include="MSTest.TestAdapter" Version="2.0.0" />