Skip to content

Commit

Permalink
Address feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
@jozkee
jozkee committed Mar 26, 2020
1 parent 4970a53 commit c4282f2
Show file tree
Hide file tree
Showing 8 changed files with 145 additions and 130 deletions.
7 changes: 4 additions & 3 deletions src/libraries/System.Net.Http.Json/src/System/Net/Http/Json/HttpContentJsonExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,9 @@ private static bool IsValidStructuredSyntaxJsonSuffix(ReadOnlySpan<char> mediaTy
int index = 0;
int typeLength = mediaType.IndexOf('/');

ReadOnlySpan<char> type = mediaType.Slice(index, typeLength);
if (typeLength < 0 ||
mediaType.Slice(index, typeLength).CompareTo(JsonContent.JsonType.AsSpan(), StringComparison.OrdinalIgnoreCase) != 0)
type.CompareTo(JsonContent.JsonType.AsSpan(), StringComparison.OrdinalIgnoreCase) != 0)
{
return false;
}
Expand All @@ -101,8 +102,8 @@ private static bool IsValidStructuredSyntaxJsonSuffix(ReadOnlySpan<char> mediaTy
}

index += suffixStart + 1;
ReadOnlySpan<char> mediaTypeSuffix = mediaType.Slice(index);
if (mediaTypeSuffix.CompareTo(JsonContent.JsonSubtype.AsSpan(), StringComparison.OrdinalIgnoreCase) != 0)
ReadOnlySpan<char> suffix = mediaType.Slice(index);
if (suffix.CompareTo(JsonContent.JsonSubtype.AsSpan(), StringComparison.OrdinalIgnoreCase) != 0)
{
return false;
}
Expand Down
7 changes: 4 additions & 3 deletions src/libraries/System.Net.Http.Json/src/System/Net/Http/Json/TranscodingReadStream.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@ internal sealed class TranscodingReadStream : Stream
{
private static readonly int OverflowBufferSize = Encoding.UTF8.GetMaxByteCount(1); // The most number of bytes used to represent a single UTF char

// Default size of the buffer that will hold the bytes from the underlying stream.
// Those bytes are expected to be encoded in the sourceEncoding passed into the .ctor.
internal const int MaxByteBufferSize = 4096;
internal const int MaxCharBufferSize = 4096;

private readonly Stream _stream;
private readonly Decoder _decoder;
Expand All @@ -39,9 +40,8 @@ public TranscodingReadStream(Stream input, Encoding sourceEncoding)

// Attempt to allocate a char buffer than can tolerate the worst-case scenario for this
// encoding. This would allow the byte -> char conversion to complete in a single call.
// However limit the buffer size to prevent an encoding that has a very poor worst-case scenario.
// The conversion process is tolerant of char buffer that is not large enough to convert all the bytes at once.
int maxCharBufferSize = Math.Min(MaxCharBufferSize, sourceEncoding.GetMaxCharCount(MaxByteBufferSize));
int maxCharBufferSize = sourceEncoding.GetMaxCharCount(MaxByteBufferSize);
_charBuffer = new ArraySegment<char>(ArrayPool<char>.Shared.Rent(maxCharBufferSize), 0, count: 0);

_overflowBuffer = new ArraySegment<byte>(ArrayPool<byte>.Shared.Rent(OverflowBufferSize), 0, count: 0);
Expand Down Expand Up @@ -124,6 +124,7 @@ private async Task<int> ReadAsyncCore(ArraySegment<byte> readBuffer, Cancellatio
bool completed = false;
int charsRead = default;
int bytesWritten = default;
// Since Convert() could fail if the destination buffer cannot fit at least one encoded char.
// If the destination buffer is smaller than GetMaxByteCount(1), we avoid encoding to the destination and we use the overflow buffer instead.
if (readBuffer.Count > OverflowBufferSize || _charBuffer.Count == 0)
{
Expand Down
2 changes: 2 additions & 0 deletions src/libraries/System.Net.Http.Json/src/System/Net/Http/Json/TranscodingWriteStream.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ namespace System.Net.Http.Json
{
internal sealed class TranscodingWriteStream : Stream
{
// Default size of the char buffer that will hold the passed-in bytes when decoded from UTF-8.
// The buffer holds them and then they are encoded to the targetEncoding and written to the underlying stream.
internal const int MaxCharBufferSize = 4096;
internal const int MaxByteBufferSize = 4 * MaxCharBufferSize;
private readonly int _maxByteBufferSize;
Expand Down
59 changes: 25 additions & 34 deletions src/libraries/System.Net.Http.Json/tests/FunctionalTests/HttpClientJsonExtensionsTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,17 +89,17 @@ await LoopbackServer.CreateClientAndServerAsync(
{
Person person = Person.Create();
HttpResponseMessage response = await client.PostAsJsonAsync(uri.ToString(), person);
using HttpResponseMessage response = await client.PostAsJsonAsync(uri.ToString(), person);
Assert.True(response.StatusCode == HttpStatusCode.OK);
response = await client.PostAsJsonAsync(uri, person);
Assert.True(response.StatusCode == HttpStatusCode.OK);
using HttpResponseMessage response2 = await client.PostAsJsonAsync(uri, person);
Assert.True(response2.StatusCode == HttpStatusCode.OK);
response = await client.PostAsJsonAsync(uri.ToString(), person, CancellationToken.None);
Assert.True(response.StatusCode == HttpStatusCode.OK);
using HttpResponseMessage response3 = await client.PostAsJsonAsync(uri.ToString(), person, CancellationToken.None);
Assert.True(response3.StatusCode == HttpStatusCode.OK);
response = await client.PostAsJsonAsync(uri, person, CancellationToken.None);
Assert.True(response.StatusCode == HttpStatusCode.OK);
using HttpResponseMessage response4 = await client.PostAsJsonAsync(uri, person, CancellationToken.None);
Assert.True(response4.StatusCode == HttpStatusCode.OK);
}
},
async server => {
Expand All @@ -125,17 +125,17 @@ await LoopbackServer.CreateClientAndServerAsync(
Person person = Person.Create();
Type typePerson = typeof(Person);
HttpResponseMessage response = await client.PutAsJsonAsync(uri.ToString(), person);
using HttpResponseMessage response = await client.PutAsJsonAsync(uri.ToString(), person);
Assert.True(response.StatusCode == HttpStatusCode.OK);
response = await client.PutAsJsonAsync(uri, person);
Assert.True(response.StatusCode == HttpStatusCode.OK);
using HttpResponseMessage response2 = await client.PutAsJsonAsync(uri, person);
Assert.True(response2.StatusCode == HttpStatusCode.OK);
response = await client.PutAsJsonAsync(uri.ToString(), person, CancellationToken.None);
Assert.True(response.StatusCode == HttpStatusCode.OK);
using HttpResponseMessage response3 = await client.PutAsJsonAsync(uri.ToString(), person, CancellationToken.None);
Assert.True(response3.StatusCode == HttpStatusCode.OK);
response = await client.PutAsJsonAsync(uri, person, CancellationToken.None);
Assert.True(response.StatusCode == HttpStatusCode.OK);
using HttpResponseMessage response4 = await client.PutAsJsonAsync(uri, person, CancellationToken.None);
Assert.True(response4.StatusCode == HttpStatusCode.OK);
}
},
async server => {
Expand All @@ -150,31 +150,22 @@ await LoopbackServer.CreateClientAndServerAsync(
}

[Fact]
public async Task TestHttpClientIsNullAsync()
public void TestHttpClientIsNullAsync()
{
HttpClient client = null;
string uriString = "http://example.com";
Uri uri = new Uri(uriString);

ArgumentNullException ex;
ex = await Assert.ThrowsAsync<ArgumentNullException>(() => client.GetFromJsonAsync(uriString, typeof(Person)));
Assert.Equal("client", ex.ParamName);
ex = await Assert.ThrowsAsync<ArgumentNullException>(() => client.GetFromJsonAsync(uri, typeof(Person)));
Assert.Equal("client", ex.ParamName);
ex = await Assert.ThrowsAsync<ArgumentNullException>(() => client.GetFromJsonAsync<Person>(uriString));
Assert.Equal("client", ex.ParamName);
ex = await Assert.ThrowsAsync<ArgumentNullException>(() => client.GetFromJsonAsync<Person>(uri));
Assert.Equal("client", ex.ParamName);

ex = await Assert.ThrowsAsync<ArgumentNullException>(() => client.PostAsJsonAsync<Person>(uriString, null));
Assert.Equal("client", ex.ParamName);
ex = await Assert.ThrowsAsync<ArgumentNullException>(() => client.PostAsJsonAsync<Person>(uri, null));
Assert.Equal("client", ex.ParamName);

ex = await Assert.ThrowsAsync<ArgumentNullException>(() => client.PutAsJsonAsync<Person>(uriString, null));
Assert.Equal("client", ex.ParamName);
ex = await Assert.ThrowsAsync<ArgumentNullException>(() => client.PutAsJsonAsync<Person>(uri, null));
Assert.Equal("client", ex.ParamName);
AssertExtensions.Throws<ArgumentNullException>("client", () => client.GetFromJsonAsync(uriString, typeof(Person)));
AssertExtensions.Throws<ArgumentNullException>("client", () => client.GetFromJsonAsync(uri, typeof(Person)));
AssertExtensions.Throws<ArgumentNullException>("client", () => client.GetFromJsonAsync<Person>(uriString));
AssertExtensions.Throws<ArgumentNullException>("client", () => client.GetFromJsonAsync<Person>(uri));

AssertExtensions.Throws<ArgumentNullException>("client", () => client.PostAsJsonAsync<Person>(uriString, null));
AssertExtensions.Throws<ArgumentNullException>("client", () => client.PostAsJsonAsync<Person>(uri, null));

AssertExtensions.Throws<ArgumentNullException>("client", () => client.PutAsJsonAsync<Person>(uriString, null));
AssertExtensions.Throws<ArgumentNullException>("client", () => client.PutAsJsonAsync<Person>(uri, null));
}

private void ValidateRequest(HttpRequestData requestData)
Expand Down
26 changes: 13 additions & 13 deletions src/libraries/System.Net.Http.Json/tests/FunctionalTests/HttpContentJsonExtensionsTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@ public class HttpContentJsonExtensionsTests
private readonly List<HttpHeaderData> _headers = new List<HttpHeaderData> { new HttpHeaderData("Content-Type", "application/json") };

[Fact]
public async Task ThrowOnNull()
public void ThrowOnNull()
{
HttpContent content = null;
await Assert.ThrowsAsync<ArgumentNullException>(() => content.ReadFromJsonAsync<Person>());
await Assert.ThrowsAsync<ArgumentNullException>(() => content.ReadFromJsonAsync(typeof(Person)));
AssertExtensions.Throws<ArgumentNullException>("content", () => content.ReadFromJsonAsync<Person>());
AssertExtensions.Throws<ArgumentNullException>("content", () => content.ReadFromJsonAsync(typeof(Person)));
}

[Fact]
Expand All @@ -35,7 +35,7 @@ await LoopbackServer.CreateClientAndServerAsync(
using (HttpClient client = new HttpClient())
{
var request = new HttpRequestMessage(HttpMethod.Get, uri);
var response = await client.SendAsync(request);
HttpResponseMessage response = await client.SendAsync(request);
object obj = await response.Content.ReadFromJsonAsync(typeof(Person));
Person per = Assert.IsType<Person>(obj);
per.Validate();
Expand Down Expand Up @@ -64,7 +64,7 @@ await LoopbackServer.CreateClientAndServerAsync(
using (HttpClient client = new HttpClient())
{
var request = new HttpRequestMessage(HttpMethod.Get, uri);
var response = await client.SendAsync(request);
HttpResponseMessage response = await client.SendAsync(request);
object obj = await response.Content.ReadFromJsonAsync(typeof(Person));
Assert.Null(obj);
Expand All @@ -91,7 +91,7 @@ await LoopbackServer.CreateClientAndServerAsync(
using (HttpClient client = new HttpClient())
{
var request = new HttpRequestMessage(HttpMethod.Get, uri);
var response = await client.SendAsync(request);
HttpResponseMessage response = await client.SendAsync(request);
// As of now, we pass the message body to the serializer even when its empty which causes the serializer to throw.
JsonException ex = await Assert.ThrowsAsync<JsonException>(() => response.Content.ReadFromJsonAsync(typeof(Person)));
Expand All @@ -110,7 +110,7 @@ await LoopbackServer.CreateClientAndServerAsync(
using (HttpClient client = new HttpClient())
{
var request = new HttpRequestMessage(HttpMethod.Get, uri);
var response = await client.SendAsync(request);
HttpResponseMessage response = await client.SendAsync(request);
await Assert.ThrowsAsync<NotSupportedException>(() => response.Content.ReadFromJsonAsync<Person>());
}
Expand All @@ -132,7 +132,7 @@ await LoopbackServer.CreateClientAndServerAsync(
using (HttpClient client = new HttpClient())
{
var request = new HttpRequestMessage(HttpMethod.Get, uri);
var response = await client.SendAsync(request);
HttpResponseMessage response = await client.SendAsync(request);
Person person = await response.Content.ReadFromJsonAsync<Person>();
person.Validate();
Expand All @@ -155,7 +155,7 @@ await LoopbackServer.CreateClientAndServerAsync(
using (HttpClient client = new HttpClient())
{
var request = new HttpRequestMessage(HttpMethod.Get, uri);
var response = await client.SendAsync(request);
HttpResponseMessage response = await client.SendAsync(request);
InvalidOperationException ex = await Assert.ThrowsAsync<InvalidOperationException>(() => response.Content.ReadFromJsonAsync<Person>());
Assert.IsType<ArgumentException>(ex.InnerException);
Expand All @@ -175,7 +175,7 @@ await LoopbackServer.CreateClientAndServerAsync(
using (HttpClient client = new HttpClient())
{
var request = new HttpRequestMessage(HttpMethod.Get, uri);
var response = await client.SendAsync(request);
HttpResponseMessage response = await client.SendAsync(request);
Person per = Assert.IsType<Person>(await response.Content.ReadFromJsonAsync(typeof(Person)));
per.Validate();
Expand Down Expand Up @@ -218,7 +218,7 @@ await LoopbackServer.CreateClientAndServerAsync(
using (HttpClient client = new HttpClient())
{
var request = new HttpRequestMessage(HttpMethod.Get, uri);
var response = await client.SendAsync(request);
HttpResponseMessage response = await client.SendAsync(request);
await response.Content.ReadFromJsonAsync(typeof(EnsureDefaultOptions));
}
},
Expand All @@ -244,7 +244,7 @@ await LoopbackServer.CreateClientAndServerAsync(
using (HttpClient client = new HttpClient())
{
var request = new HttpRequestMessage(HttpMethod.Get, uri);
var response = await client.SendAsync(request);
HttpResponseMessage response = await client.SendAsync(request);
Person person = await response.Content.ReadFromJsonAsync<Person>();
person.Validate();
Expand Down Expand Up @@ -274,7 +274,7 @@ await LoopbackServer.CreateClientAndServerAsync(
using (HttpClient client = new HttpClient())
{
var request = new HttpRequestMessage(HttpMethod.Get, uri);
var response = await client.SendAsync(request);
HttpResponseMessage response = await client.SendAsync(request);
Exception ex = await Assert.ThrowsAsync<NotSupportedException>(() => response.Content.ReadFromJsonAsync<Person>());
Assert.Contains("application/json", ex.Message);
Expand Down
Loading

0 comments on commit c4282f2

Please sign in to comment.