Max length check/handling for SyncTriggers

Author: mathewc

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

@@ -92,8 +92,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;
@@ -229,7 +228,7 @@ internal CloudBlockBlob GetHashBlob()
             return _hashBlob;
         }
 
-        public async Task<JToken> GetSyncTriggersPayload()
+        public async Task<string> GetSyncTriggersPayload()
         {
             var functionsMetadata = WebFunctionsManager.GetFunctionsMetadata(_hostConfig, _logger);
 
@@ -240,7 +239,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
@@ -293,7 +292,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;
         }
 
         private async Task<IEnumerable<JObject>> GetFunctionTriggers(IEnumerable<FunctionMetadata> functionsMetadata)
@@ -403,9 +412,12 @@ internal static 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

@@ -100,5 +100,11 @@ public static class ScriptConstants
         public const int MaximumSecretBackupCount = 10;
 
         public const string AzureWebJobsHostsContainerName = "azure-webjobs-hosts";
+
+        /// <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;
     }
 }

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

@@ -36,6 +36,7 @@ public class FunctionsSyncManagerTests : IDisposable
         private readonly MockHttpHandler _mockHttpHandler;
         private readonly TestLoggerProvider _loggerProvider;
         private readonly Mock<ScriptSettingsManager> _scriptSettingsManagerMock;
+        private string _function1;
 
         public FunctionsSyncManagerTests()
         {
@@ -142,6 +143,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()
         {
@@ -421,7 +443,7 @@ private static HttpClient CreateHttpClient(MockHttpHandler httpHandler)
             return new HttpClient(httpHandler);
         }
 
-        private static IFileSystem CreateFileSystem(ScriptHostConfiguration hostConfig, string hostJsonContent = null)
+        private IFileSystem CreateFileSystem(ScriptHostConfiguration hostConfig, string hostJsonContent = null)
         {
             string rootScriptPath = hostConfig.RootScriptPath;
             string testDataPath = hostConfig.TestDataPath;
@@ -450,7 +472,7 @@ private static IFileSystem CreateFileSystem(ScriptHostConfiguration hostConfig,
                     Path.Combine(rootScriptPath, "function3")
                 });
 
-            var function1 = @"{
+            _function1 = @"{
   ""scriptFile"": ""main.py"",
   ""disabled"": false,
   ""bindings"": [
@@ -497,14 +519,14 @@ private static IFileSystem CreateFileSystem(ScriptHostConfiguration hostConfig,
 
             fileBase.Setup(f => f.Exists(Path.Combine(rootScriptPath, @"function1\function.json"))).Returns(true);
             fileBase.Setup(f => f.Exists(Path.Combine(rootScriptPath, @"function1\main.py"))).Returns(true);
-            fileBase.Setup(f => f.ReadAllText(Path.Combine(rootScriptPath, @"function1\function.json"))).Returns(function1);
+            fileBase.Setup(f => f.ReadAllText(Path.Combine(rootScriptPath, @"function1\function.json"))).Returns(_function1);
             fileBase.Setup(f => f.Open(Path.Combine(rootScriptPath, @"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(rootScriptPath, @"function2\function.json"))).Returns(true);
@@ -516,7 +538,7 @@ private static IFileSystem CreateFileSystem(ScriptHostConfiguration hostConfig,
             });
             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(rootScriptPath, @"function3\function.json"))).Returns(true);
@@ -528,7 +550,7 @@ private static IFileSystem CreateFileSystem(ScriptHostConfiguration hostConfig,
             });
             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;