Skip to content

Commit c7dbcde

Browse files
authored
[AppConfig] Fix disposed stream access (Azure#38531)
The focus of these changes is to fix a bug where the content stream returned by the protocol method has been disposed and the serializer attempts to access it. Shifting to accessing the `BinaryData` content directly for deserialization.
1 parent 8dca6d7 commit c7dbcde

File tree

4 files changed

+66
-16
lines changed

4 files changed

+66
-16
lines changed

sdk/appconfiguration/Azure.Data.AppConfiguration/CHANGELOG.md

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,16 @@
44

55
### Bugs Fixed
66

7-
- `FeatureFlagConfigurationSetting` and `SecretReferenceConfigurationSetting` will now retain custom attributes in the setting value. Previously, only attributes that were defined in the associated JSON schema were allowed and unknown attributes were discarded.
7+
- `FeatureFlagConfigurationSetting` and `SecretReferenceConfigurationSetting` will now retain custom attributes in the setting value. Previously, only attributes that were defined in the associated JSON schema were allowed and unknown attributes were discarded.
88

99
- Added the ability to create `FeatureFlagConfigurationSetting` and `SecretReferenceConfigurationSetting` instances with an ETag, matching `ConfigurationSetting`. This allows all setting types to use the [GetConfigurationSettingAsync](https://learn.microsoft.com/dotnet/api/azure.data.appconfiguration.configurationclient.getconfigurationsettingasync?view=azure-dotnet#azure-data-appconfiguration-configurationclient-getconfigurationsettingasync(azure-data-appconfiguration-configurationsetting-system-boolean-system-threading-cancellationtoken)) overload that accepts `onlyIfUnchanged.` Previously, this was not possible for specialized settings types.
1010

1111
- Added the ability to create `FeatureFlagConfigurationSetting` and `SecretReferenceConfigurationSetting` instances for testing purposes using the `ConfigurationModelFactory`. It was previously not possible to populate service-owned fields when testing.
1212

13+
- Marked a constructor overload of `ConfigurationSetting` that was intended for testing purposes as non-visible, as the `ConfigurationModelFactory` should instead be used.
14+
15+
- Fixed a bug where a disposed content stream was used to attempt deserialization in some scenarios, such as using a custom `HttpMessageHandler` that returns `StringContent`.
16+
1317
## 1.3.0-beta.2 (2023-07-11)
1418

1519
### Features Added
@@ -77,7 +81,7 @@
7781

7882
#### New Features
7983

80-
- Added `SecretReferenceConfigurationSetting` type to represent a configuration setting that references a KeyVault Secret.
84+
- Added `SecretReferenceConfigurationSetting` type to represent a configuration setting that references a KeyVault Secret.
8185
- Added `FeatureFlagConfigurationSetting` type to represent a configuration setting that controls a feature flag.
8286
- Added `AddSyncToken` to `ConfigurationClient` to be able to provide external synchronization tokens.
8387

@@ -89,7 +93,7 @@
8993

9094
- Update the tag list for the AzConfig package
9195

92-
## 1.0.0
96+
## 1.0.0
9397

9498
### Breaking changes
9599

@@ -99,11 +103,11 @@
99103

100104
- Fixed multiple issues with connection string parsing in `ConfigurationClient`.
101105

102-
## 1.0.0-preview.6
106+
## 1.0.0-preview.6
103107

104108
- Bugfixes: [#8920](https://github.com/Azure/azure-sdk-for-net/issues/8920)
105109

106-
## 1.0.0-preview.5
110+
## 1.0.0-preview.5
107111

108112
### Breaking changes
109113

@@ -116,7 +120,7 @@
116120
- Added new overload for the method `ConfigurationClient.GetRevisions` that accepts key and optional label.
117121
- Added new overload for the method `ConfigurationClient.GetConfigurationSetting` that accepts `ConfigurationSetting` and its datetime stamp.
118122

119-
## 1.0.0-preview.4
123+
## 1.0.0-preview.4
120124

121125
### Breaking changes
122126

@@ -135,11 +139,11 @@
135139
- Made `ConfigurationSetting` serializable by `System.Text.Json` serializers.
136140
- Updated documentation and samples.
137141

138-
## 1.0.0-preview.3
142+
## 1.0.0-preview.3
139143

140144
- Fixed an issue where special characters were escaped incorrectly.
141145

142-
## 1.0.0-preview.2
146+
## 1.0.0-preview.2
143147

144148
- Enabled conditional requests.
145149
- Added support for setting `x-ms-client-request-id`, `x-ms-correlation-request-id`, and `correlation-context` headers.

sdk/appconfiguration/Azure.Data.AppConfiguration/src/ConfigurationClient_private.cs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,8 @@
22
// Licensed under the MIT License.
33

44
using System;
5-
using System.Collections.Generic;
65
using System.ComponentModel;
76
using System.Diagnostics;
8-
using System.Linq;
97
using System.Threading;
108
using System.Threading.Tasks;
119
using Azure.Core;
@@ -21,13 +19,13 @@ public partial class ConfigurationClient
2119

2220
private static async Task<Response<ConfigurationSetting>> CreateResponseAsync(Response response, CancellationToken cancellation)
2321
{
24-
ConfigurationSetting result = await ConfigurationServiceSerializer.DeserializeSettingAsync(response.ContentStream, cancellation).ConfigureAwait(false);
22+
ConfigurationSetting result = await ConfigurationServiceSerializer.DeserializeSettingAsync(response.Content, cancellation).ConfigureAwait(false);
2523
return Response.FromValue(result, response);
2624
}
2725

2826
private static Response<ConfigurationSetting> CreateResponse(Response response)
2927
{
30-
return Response.FromValue(ConfigurationServiceSerializer.DeserializeSetting(response.ContentStream), response);
28+
return Response.FromValue(ConfigurationServiceSerializer.DeserializeSetting(response.Content), response);
3129
}
3230

3331
private static Response<ConfigurationSetting> CreateResourceModifiedResponse(Response response)

sdk/appconfiguration/Azure.Data.AppConfiguration/src/Models/ConfigurationServiceSerializer.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -169,18 +169,18 @@ private static void ReadPropertyValue(ConfigurationSetting setting, JsonProperty
169169
}
170170
}
171171

172-
public static async Task<ConfigurationSetting> DeserializeSettingAsync(Stream content, CancellationToken cancellation)
172+
public static async Task<ConfigurationSetting> DeserializeSettingAsync(BinaryData content, CancellationToken cancellation)
173173
{
174-
using (JsonDocument json = await JsonDocument.ParseAsync(content, default, cancellation).ConfigureAwait(false))
174+
using (JsonDocument json = await JsonDocument.ParseAsync(content.ToStream(), default, cancellation).ConfigureAwait(false))
175175
{
176176
JsonElement root = json.RootElement;
177177
return ReadSetting(root);
178178
}
179179
}
180180

181-
public static ConfigurationSetting DeserializeSetting(Stream content)
181+
public static ConfigurationSetting DeserializeSetting(BinaryData content)
182182
{
183-
using JsonDocument json = JsonDocument.Parse(content, default);
183+
using JsonDocument json = JsonDocument.Parse(content.ToMemory(), default);
184184
JsonElement root = json.RootElement;
185185
return ReadSetting(root);
186186
}

sdk/appconfiguration/Azure.Data.AppConfiguration/tests/ConfigurationMockTests.cs

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77
using System.Globalization;
88
using System.IO;
99
using System.Linq;
10+
using System.Net;
11+
using System.Net.Http;
1012
using System.Text;
1113
using System.Text.Json;
1214
using System.Text.RegularExpressions;
@@ -870,6 +872,33 @@ public async Task VerifyNullClientFilter()
870872
Assert.IsEmpty(feature.ClientFilters);
871873
}
872874

875+
[Test]
876+
public async Task SupportsCustomTransportUse()
877+
{
878+
var expectedKey = "abc";
879+
var expectedValue = "ghi";
880+
var expectedLabel = "def";
881+
var expectedContent = @$"{{""key"":""{expectedKey}"",""label"":""{expectedLabel}"",""value"":""{expectedValue}""}}";
882+
883+
var client = new ConfigurationClient(
884+
s_connectionString,
885+
new ConfigurationClientOptions
886+
{
887+
Transport = new HttpClientTransport(new EchoHttpMessageHandler(expectedContent))
888+
}
889+
);
890+
891+
var result = await client.GetConfigurationSettingAsync("doesnt-matter");
892+
Assert.AreEqual(expectedKey, result.Value.Key);
893+
Assert.AreEqual(expectedValue, result.Value.Value);
894+
Assert.AreEqual(expectedLabel, result.Value.Label);
895+
896+
var result2 = await client.SetConfigurationSettingAsync("whatever", "somevalue");
897+
Assert.AreEqual(expectedKey, result.Value.Key);
898+
Assert.AreEqual(expectedValue, result.Value.Value);
899+
Assert.AreEqual(expectedLabel, result.Value.Label);
900+
}
901+
873902
private void AssertContent(byte[] expected, MockRequest request, bool compareAsString = true)
874903
{
875904
using (var stream = new MemoryStream())
@@ -961,5 +990,24 @@ private void SerializeBatch(ref Utf8JsonWriter json, (ConfigurationSetting[] Set
961990
json.WriteEndArray();
962991
json.WriteEndObject();
963992
}
993+
994+
private class EchoHttpMessageHandler : HttpMessageHandler
995+
{
996+
private readonly string _expectedContent;
997+
998+
public EchoHttpMessageHandler(string expectedJsonContent)
999+
{
1000+
_expectedContent = expectedJsonContent;
1001+
}
1002+
1003+
protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
1004+
{
1005+
return Task.FromResult(new HttpResponseMessage()
1006+
{
1007+
StatusCode = HttpStatusCode.OK,
1008+
Content = new StringContent(_expectedContent, Encoding.UTF8, "application/json")
1009+
});
1010+
}
1011+
}
9641012
}
9651013
}

0 commit comments

Comments
 (0)