Skip to content

Commit 1b82fb8

Browse files
Aniruddh25abhishekkumams
Aniruddh25
and
abhishekkumams
authored
Avoid escaping HTML sensitive characters when writing configuration to file (#1691)
## Why make this change? - Closes #1687 ## What is this change? - Set the Encoder for JsonSerializationOptions to be [`JavaScriptEncoder.UnsafeRelaxedJsonEscaping`](https://learn.microsoft.com/en-us/dotnet/api/system.text.encodings.web.javascriptencoder.unsaferelaxedjsonescaping?view=net-7.0) instead of the default encoder. - Although [this article](https://learn.microsoft.com/en-us/dotnet/standard/serialization/system-text-json/character-encoding#serialize-all-characters) mentions use this encoder with caution, since we are sure the file contents will be deserialized as UTF-8 JSON, we should be safe to use this encoder. - We were using this encoder previously from the fix in #1386, but with #1402, that original fix was essentially not being used since #1402 introduced usage of the new `GetJsonSerializerOptions()` function in `RuntimeConfigLoader`. ## How was this tested? - [X] Unit Tests. `TestSpecialCharactersInConnectionString` is moved to the `ConfigGeneratorTests` since keeping it as part of `InitTests` with the Verify framework wasn't really testing the scenario since we ignore connection strings in the snapshots. - This regression from #1402 was not caught because this test scenario was effectively being skipped. - Note, this test is modified to explicitly compare the expected and actual json string from file since any usage of JsonSerializer/JObject.Parse would require using the same encoder to deserialize making the test itself run similar code which it is testing. Parsing into JSON objects would treat `\u0027` and `'` as equal whereas string comparison wont. Hence, comparison using string is necessary to catch regressions here. ## Sample Request(s) BEFORE: `dab init --host-mode development --database-type mssql --connection-string "@env('my-connection-string')"` creates the configuration file as follows: ``` { "$schema": "https://github.com/Azure/data-api-builder/releases/download/v0.8.49/dab.draft.schema.json", "data-source": { "database-type": "mssql", "connection-string": "@env(\u0027my-connection-string\u0027)", "options": { "set-session-context": false } } ``` Note, the value of connection string. AFTER: Same command creates the following file: ``` { "$schema": "https://github.com/Azure/data-api-builder/releases/download/v0.8.49/dab.draft.schema.json", "data-source": { "database-type": "mssql", "connection-string": "@env('my-connection-string')", "options": { "set-session-context": false } } ``` --------- Co-authored-by: Abhishek Kumar <[email protected]>
1 parent b5d2cb6 commit 1b82fb8

4 files changed

Lines changed: 76 additions & 49 deletions

File tree

‎src/Cli.Tests/ConfigGeneratorTests.cs‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
// Copyright (c) Microsoft Corporation.
22
// Licensed under the MIT License.
33

4+
using System.Text;
5+
46
namespace Cli.Tests;
57

68
/// <summary>
@@ -119,6 +121,77 @@ public void TryGenerateConfig_UsingEnvironmentVariable(
119121
}
120122
}
121123

124+
/// <summary>
125+
/// Test to verify creation of initial config with special characters
126+
/// such as [!,@,#,$,%,^,&,*, ,(,)] in connection-string or graphql api
127+
/// </summary>
128+
[TestMethod]
129+
public void TestSpecialCharactersInConnectionString()
130+
{
131+
HandleConfigFileCreationAndDeletion(TEST_RUNTIME_CONFIG_FILE, configFilePresent: false);
132+
InitOptions options = new(
133+
databaseType: DatabaseType.MSSQL,
134+
connectionString: "A!string@with#some$special%characters^to&check*proper(serialization).'",
135+
cosmosNoSqlDatabase: null,
136+
cosmosNoSqlContainer: null,
137+
graphQLSchemaPath: null,
138+
graphQLPath: "/An_",
139+
setSessionContext: false,
140+
hostMode: HostMode.Production,
141+
corsOrigin: null,
142+
authenticationProvider: EasyAuthType.StaticWebApps.ToString(),
143+
config: TEST_RUNTIME_CONFIG_FILE);
144+
145+
StringBuilder expectedRuntimeConfigJson = new(
146+
@"{" +
147+
@"""$schema"": """ + DAB_DRAFT_SCHEMA_TEST_PATH + @"""" + "," +
148+
@"""data-source"": {
149+
""database-type"": ""mssql"",
150+
""connection-string"": ""A!string@with#some$special%characters^to&check*proper(serialization).'"",
151+
""options"":{
152+
""set-session-context"": false
153+
}
154+
},
155+
""runtime"": {
156+
""rest"": {
157+
""enabled"": true,
158+
""path"": ""/api""
159+
},
160+
""graphql"": {
161+
""enabled"": true,
162+
""path"": ""/An_"",
163+
""allow-introspection"": true
164+
},
165+
""host"": {
166+
""cors"": {
167+
""origins"": [],
168+
""allow-credentials"": false
169+
},
170+
""authentication"": {
171+
""provider"": ""StaticWebApps""
172+
},
173+
""mode"": ""production""
174+
}
175+
},
176+
""entities"": {}
177+
}");
178+
179+
expectedRuntimeConfigJson = expectedRuntimeConfigJson.Replace(" ", string.Empty);
180+
expectedRuntimeConfigJson = expectedRuntimeConfigJson.Replace("\n", string.Empty);
181+
expectedRuntimeConfigJson = expectedRuntimeConfigJson.Replace("\r\n", string.Empty);
182+
183+
Assert.IsTrue(TryGenerateConfig(options, _runtimeConfigLoader!, _fileSystem!));
184+
185+
StringBuilder actualRuntimeConfigJson = new(_fileSystem!.File.ReadAllText(TEST_RUNTIME_CONFIG_FILE, Encoding.Default));
186+
actualRuntimeConfigJson = actualRuntimeConfigJson.Replace(" ", string.Empty);
187+
actualRuntimeConfigJson = actualRuntimeConfigJson.Replace("\r\n", string.Empty);
188+
actualRuntimeConfigJson = actualRuntimeConfigJson.Replace("\n", string.Empty);
189+
190+
// Comparing explicit strings here since parsing these into JSON would lose
191+
// the test scenario of verifying escaped chars are not written to the file system.
192+
Assert.AreEqual(expectedRuntimeConfigJson.ToString(), actualRuntimeConfigJson.ToString());
193+
}
194+
122195
/// <summary>
123196
/// This method handles the creation and deletion of a configuration file.
124197
/// </summary>

‎src/Cli.Tests/InitTests.cs‎

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -223,28 +223,6 @@ public void EnsureFailureWhenBothRestAndGraphQLAreDisabled(
223223
Assert.AreEqual(expectedResult, TryCreateRuntimeConfig(options, _runtimeConfigLoader!, _fileSystem!, out RuntimeConfig? _));
224224
}
225225

226-
/// <summary>
227-
/// Test to verify creation of initial config with special characters
228-
/// such as [!,@,#,$,%,^,&,*, ,(,)] in connection-string.
229-
/// </summary>
230-
[TestMethod]
231-
public Task TestSpecialCharactersInConnectionString()
232-
{
233-
InitOptions options = new(
234-
databaseType: DatabaseType.MSSQL,
235-
connectionString: "A!string@with#some$special%characters^to&check*proper(serialization)including space.",
236-
cosmosNoSqlDatabase: null,
237-
cosmosNoSqlContainer: null,
238-
graphQLSchemaPath: null,
239-
setSessionContext: false,
240-
hostMode: HostMode.Production,
241-
corsOrigin: null,
242-
authenticationProvider: EasyAuthType.StaticWebApps.ToString(),
243-
config: TEST_RUNTIME_CONFIG_FILE);
244-
245-
return ExecuteVerifyTest(options);
246-
}
247-
248226
/// <summary>
249227
/// Test to verify that an error is thrown when user tries to
250228
/// initialize a config with a file name that already exists.

‎src/Cli/Utils.cs‎

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,7 @@
33

44
using System.Diagnostics.CodeAnalysis;
55
using System.IO.Abstractions;
6-
using System.Text.Encodings.Web;
76
using System.Text.Json;
8-
using System.Text.Json.Serialization;
97
using Azure.DataApiBuilder.Config;
108
using Azure.DataApiBuilder.Config.Converters;
119
using Azure.DataApiBuilder.Config.ObjectModel;
@@ -132,30 +130,6 @@ public class LowerCaseNamingPolicy : JsonNamingPolicy
132130
public static string ConvertName(Enum name) => name.ToString().ToLower();
133131
}
134132

135-
/// <summary>
136-
/// Returns the Serialization option used to convert objects into JSON.
137-
/// Not escaping any special unicode characters.
138-
/// Ignoring properties with null values.
139-
/// Keeping all the keys in lowercase.
140-
/// </summary>
141-
public static JsonSerializerOptions GetSerializationOptions()
142-
{
143-
JsonSerializerOptions? options = new()
144-
{
145-
Encoder = JavaScriptEncoder.UnsafeRelaxedJsonEscaping,
146-
WriteIndented = true,
147-
DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull,
148-
PropertyNamingPolicy = new LowerCaseNamingPolicy(),
149-
// As of .NET Core 7, JsonDocument and JsonSerializer only support skipping or disallowing
150-
// of comments; they do not support loading them. If we set JsonCommentHandling.Allow for either,
151-
// it will throw an exception.
152-
ReadCommentHandling = JsonCommentHandling.Skip
153-
};
154-
155-
options.Converters.Add(new JsonStringEnumConverter(namingPolicy: new LowerCaseNamingPolicy()));
156-
return options;
157-
}
158-
159133
/// <summary>
160134
/// Returns true on successful parsing of mappings Dictionary from IEnumerable list.
161135
/// Returns false in case the format of the input is not correct.

‎src/Config/RuntimeConfigLoader.cs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
using System.Diagnostics.CodeAnalysis;
55
using System.Net;
66
using System.Runtime.CompilerServices;
7+
using System.Text.Encodings.Web;
78
using System.Text.Json;
89
using System.Text.Json.Serialization;
910
using Azure.DataApiBuilder.Config.Converters;
@@ -155,7 +156,8 @@ public static JsonSerializerOptions GetSerializationOptions(bool replaceEnvVar =
155156
PropertyNamingPolicy = new HyphenatedNamingPolicy(),
156157
ReadCommentHandling = JsonCommentHandling.Skip,
157158
WriteIndented = true,
158-
DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull
159+
DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull,
160+
Encoder = JavaScriptEncoder.UnsafeRelaxedJsonEscaping
159161
};
160162
options.Converters.Add(new EnumMemberJsonEnumConverterFactory());
161163
options.Converters.Add(new RestRuntimeOptionsConverterFactory());

0 commit comments

Comments
 (0)