Max length check/handling for SyncTriggers

Author: mathewc

src/WebJobs.Script.WebHost/Management/FunctionsSyncManager.cs

@@ -97,8 +97,7 @@ public async Task<SyncTriggersResult> TrySyncTriggersAsync(bool checkHash = fals
                     return result;
                 }
 
-                var payload = await GetSyncTriggersPayload();
-                string json = JsonConvert.SerializeObject(payload);
+                string json = await GetSyncTriggersPayload();
 
                 bool shouldSyncTriggers = true;
                 string newHash = null;
@@ -245,7 +244,7 @@ internal async Task<CloudBlockBlob> GetHashBlobAsync()
             return _hashBlob;
         }
 
-        public async Task<JToken> GetSyncTriggersPayload()
+        public async Task<string> GetSyncTriggersPayload()
         {
             var hostOptions = _applicationHostOptions.CurrentValue.ToHostOptions();
             var functionsMetadata = WebFunctionsManager.GetFunctionsMetadata(hostOptions, _workerConfigs, _logger);
@@ -257,7 +256,7 @@ public async Task<JToken> GetSyncTriggersPayload()
             if (!ArmCacheEnabled)
             {
                 // extended format is disabled - just return triggers
-                return triggersArray;
+                return JsonConvert.SerializeObject(triggersArray);
             }
 
             // Add triggers to the payload
@@ -310,7 +309,17 @@ public async Task<JToken> GetSyncTriggersPayload()
                 // like KeyVault when the feature comes online
             }
 
-            return result;
+            string json = JsonConvert.SerializeObject(result);
+            if (json.Length > ScriptConstants.MaxTriggersStringLength)
+            {
+                // The settriggers call to the FE enforces a max request size
+                // limit. If we're over limit, revert to the minimal triggers
+                // format.
+                _logger.LogWarning($"SyncTriggers payload of length '{json.Length}' exceeds max length of '{ScriptConstants.MaxTriggersStringLength}'. Reverting to minimal format.");
+                return JsonConvert.SerializeObject(triggersArray);
+            }
+
+            return json;
         }
 
         internal async Task<IEnumerable<JObject>> GetFunctionTriggers(IEnumerable<FunctionMetadata> functionsMetadata, ScriptJobHostOptions hostOptions)
@@ -420,9 +429,12 @@ internal HttpRequestMessage BuildSetTriggersRequest()
             if (ArmCacheEnabled)
             {
                 // sanitize the content before logging
-                var sanitizedContent = JObject.Parse(content);
-                sanitizedContent.Remove("secrets");
-                sanitizedContentString = sanitizedContent.ToString();
+                var sanitizedContent = JToken.Parse(content);
+                if (sanitizedContent.Type == JTokenType.Object)
+                {
+                    ((JObject)sanitizedContent).Remove("secrets");
+                    sanitizedContentString = sanitizedContent.ToString();
+                }
             }
 
             using (var request = BuildSetTriggersRequest())

src/WebJobs.Script/ScriptConstants.cs

@@ -106,6 +106,12 @@ public static class ScriptConstants
         public const int MaximumHostIdLength = 32;
         public const int DynamicSkuConnectionLimit = 50;
 
+        /// <summary>
+        /// This constant is also defined in Antares, where the limit is ultimately enforced
+        /// for settriggers calls. If we raise that limit there, we should raise here as well.
+        /// </summary>
+        public const int MaxTriggersStringLength = 102400;
+
         public const string ExtensionsProjectFileName = "extensions.csproj";
         public const string MetadataGeneratorPackageId = "Microsoft.Azure.WebJobs.Script.ExtensionsMetadataGenerator";
         public const string MetadataGeneratorPackageVersion = "1.1.*";

test/WebJobs.Script.Tests/Managment/FunctionsSyncManagerTests.cs

@@ -40,6 +40,7 @@ public class FunctionsSyncManagerTests : IDisposable
         private readonly Mock<IScriptWebHostEnvironment> _mockWebHostEnvironment;
         private readonly Mock<IEnvironment> _mockEnvironment;
         private readonly HostNameProvider _hostNameProvider;
+        private string _function1;
 
         public FunctionsSyncManagerTests()
         {
@@ -139,6 +140,27 @@ public async Task TrySyncTriggers_StandbyMode_ReturnsFalse()
             }
         }
 
+        [Fact]
+        public async Task TrySyncTriggers_MaxSyncTriggersPayloadSize_Succeeds()
+        {
+            // create a dummy file that pushes us over size
+            string maxString = new string('x', ScriptConstants.MaxTriggersStringLength + 1);
+            _function1 = $"{{ bindings: [], test: '{maxString}'}}";
+
+            using (var env = new TestScopedEnvironmentVariable(_vars))
+            {
+                Assert.True(_functionsSyncManager.ArmCacheEnabled);
+
+                var result = await _functionsSyncManager.TrySyncTriggersAsync();
+                Assert.True(result.Success);
+
+                string syncString = _contentBuilder.ToString();
+                Assert.True(syncString.Length < ScriptConstants.MaxTriggersStringLength);
+                var syncContent = JToken.Parse(syncString);
+                Assert.Equal(JTokenType.Array, syncContent.Type);
+            }
+        }
+
         [Fact]
         public async Task TrySyncTriggers_LocalEnvironment_ReturnsFalse()
         {
@@ -370,9 +392,9 @@ private static LanguageWorkerOptions CreateLanguageWorkerConfigSettings()
             };
         }
 
-        private static IFileSystem CreateFileSystem(ScriptApplicationHostOptions hostOptions, string hostJsonContent = null)
+        private IFileSystem CreateFileSystem(ScriptApplicationHostOptions hostOptions, string hostJsonContent = null)
         {
-            string rootPath = hostOptions.ScriptPath;
+            var rootPath = hostOptions.ScriptPath;
             string testDataPath = hostOptions.TestDataPath;
 
             var fullFileSystem = new FileSystem();
@@ -412,7 +434,7 @@ private static IFileSystem CreateFileSystem(ScriptApplicationHostOptions hostOpt
                     Path.Combine(rootPath, "function3")
                 });
 
-            var function1 = @"{
+            _function1 = @"{
   ""scriptFile"": ""main.py"",
   ""disabled"": false,
   ""bindings"": [
@@ -459,14 +481,14 @@ private static IFileSystem CreateFileSystem(ScriptApplicationHostOptions hostOpt
 
             fileBase.Setup(f => f.Exists(Path.Combine(rootPath, @"function1\function.json"))).Returns(true);
             fileBase.Setup(f => f.Exists(Path.Combine(rootPath, @"function1\main.py"))).Returns(true);
-            fileBase.Setup(f => f.ReadAllText(Path.Combine(rootPath, @"function1\function.json"))).Returns(function1);
+            fileBase.Setup(f => f.ReadAllText(Path.Combine(rootPath, @"function1\function.json"))).Returns(_function1);
             fileBase.Setup(f => f.Open(Path.Combine(rootPath, @"function1\function.json"), It.IsAny<FileMode>(), It.IsAny<FileAccess>(), It.IsAny<FileShare>())).Returns(() =>
             {
-                return new MemoryStream(Encoding.UTF8.GetBytes(function1));
+                return new MemoryStream(Encoding.UTF8.GetBytes(_function1));
             });
             fileBase.Setup(f => f.Open(Path.Combine(testDataPath, "function1.dat"), It.IsAny<FileMode>(), It.IsAny<FileAccess>(), It.IsAny<FileShare>())).Returns(() =>
             {
-                return new MemoryStream(Encoding.UTF8.GetBytes(function1));
+                return new MemoryStream(Encoding.UTF8.GetBytes(_function1));
             });
 
             fileBase.Setup(f => f.Exists(Path.Combine(rootPath, @"function2\function.json"))).Returns(true);
@@ -478,7 +500,7 @@ private static IFileSystem CreateFileSystem(ScriptApplicationHostOptions hostOpt
             });
             fileBase.Setup(f => f.Open(Path.Combine(testDataPath, "function2.dat"), It.IsAny<FileMode>(), It.IsAny<FileAccess>(), It.IsAny<FileShare>())).Returns(() =>
             {
-                return new MemoryStream(Encoding.UTF8.GetBytes(function1));
+                return new MemoryStream(Encoding.UTF8.GetBytes(_function1));
             });
 
             fileBase.Setup(f => f.Exists(Path.Combine(rootPath, @"function3\function.json"))).Returns(true);
@@ -490,7 +512,7 @@ private static IFileSystem CreateFileSystem(ScriptApplicationHostOptions hostOpt
             });
             fileBase.Setup(f => f.Open(Path.Combine(testDataPath, "function3.dat"), It.IsAny<FileMode>(), It.IsAny<FileAccess>(), It.IsAny<FileShare>())).Returns(() =>
             {
-                return new MemoryStream(Encoding.UTF8.GetBytes(function1));
+                return new MemoryStream(Encoding.UTF8.GetBytes(_function1));
             });
 
             return fileSystem.Object;