Skip to content

Commit

Permalink
Enhance path validation logic
Browse files Browse the repository at this point in the history
  • Loading branch information
sebastienros committed Oct 5, 2023
1 parent 40c80b1 commit 2fc2bac
Show file tree
Hide file tree
Showing 4 changed files with 198 additions and 5 deletions.
30 changes: 27 additions & 3 deletions src/Generators/Microsoft.Gen.AutoClient/Emitter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ internal sealed class Emitter : EmitterBase
private const string AutoClientHttpError = "global::Microsoft.Extensions.Http.AutoClient.AutoClientHttpError";
private const string Invariant = "global::System.FormattableString.Invariant";
private const string UriKind = "global::System.UriKind";
private const string ArgumentException = "global::System.ArgumentException";
private const string AutoClientUtilities = "global::Microsoft.Extensions.Http.AutoClient.AutoClientUtilities";

public string EmitRestApis(IReadOnlyList<RestApiType> restApiTypes, CancellationToken cancellationToken)
{
Expand All @@ -67,6 +69,8 @@ public string EmitRestApis(IReadOnlyList<RestApiType> restApiTypes, Cancellation
return Capture();
}

private static string GetEscapedParamName(string name) => $"{name}Escaped";

private static string GetPathTemplate(RestApiMethod restApiMethod)
{
var pathTemplateSb = new StringBuilder(restApiMethod.Path);
Expand Down Expand Up @@ -229,10 +233,26 @@ private void GenRestApiMethod(RestApiMethod restApiMethod, RestApiType restApiTy

var pathSb = new StringBuilder(restApiMethod.Path);

// Validate path values
foreach (var param in restApiMethod.FormatParameters)
{
var escapedParamName = GetEscapedParamName(param);

// Use interpolated string to handle any null values from any type like nullable value types or reference types.
OutLn($"var {escapedParamName} = $\"{{{param}}}\";");

// Values should not contain '/', '\', be empty, whitespace or made of dots (.) only
OutLn($"if (!{AutoClientUtilities}.IsPathParameterValid({escapedParamName})) throw new {ArgumentException}({staticsName}.ValidationExceptionMessage, nameof({param}));");
OutLn();

_ = pathSb.Replace($"{{{param}}}", $"{{{escapedParamName}}}");
}

// Encode path values
foreach (var param in restApiMethod.FormatParameters)
{
var escapedParamName = $"{param}Escaped";
OutLn($"var {escapedParamName} = {Uri}.EscapeDataString($\"{{{param}}}\");");
var escapedParamName = GetEscapedParamName(param);
OutLn($"{escapedParamName} = {Uri}.EscapeDataString({escapedParamName});");

_ = pathSb.Replace($"{{{param}}}", $"{{{escapedParamName}}}");
}
Expand All @@ -242,7 +262,7 @@ private void GenRestApiMethod(RestApiMethod restApiMethod, RestApiType restApiTy
var firstQuery = true;
foreach (var param in restApiMethod.AllParameters.Where(m => m.IsQuery))
{
var escapedParamName = $"{param.Name}Escaped";
var escapedParamName = GetEscapedParamName(param.Name);

// Use interpolated string to handle any null values from any type like nullable value types or reference types.
OutLn($"var {escapedParamName} = {Uri}.EscapeDataString($\"{{{param.Name}}}\");");
Expand Down Expand Up @@ -357,6 +377,7 @@ private void GenRestApiMethod(RestApiMethod restApiMethod, RestApiType restApiTy
OutCloseBrace();

OutCloseBrace();

}

private void EmitSendRequestMethod(string httpClientName, string optionsName)
Expand Down Expand Up @@ -440,6 +461,9 @@ private void EmitStatics(RestApiType restApiType, string staticsName)
OutLn($"private static class {staticsName}");
OutOpenBrace();

OutLn("public const string ValidationExceptionMessage = \"The value can't contain '\\\\', '/', be empty, null or contain only dots (.).\";");
OutLn();

OutLn(@$"public static readonly {MediaTypeHeaderValue} ApplicationJsonHeader = new(""application/json"")");
OutOpenBrace();
OutLn($@"CharSet = {Encoding}.UTF8.WebName");
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.ComponentModel;

namespace Microsoft.Extensions.Http.AutoClient;

/// <summary>
/// Utilities for AutoClient feature.
/// </summary>
[EditorBrowsable(EditorBrowsableState.Never)]
public static class AutoClientUtilities
{
private static readonly char[] _slashes = { '/', '\\' };
private static readonly char[] _slashesOrDot = { '/', '\\', '.' };

/// <summary>
/// Returns whether a value is a valid path argument.
/// </summary>
/// <param name="value">The value to validate.</param>
/// <returns>Whether the value is a valid path argument.</returns>
public static bool IsPathParameterValid(string value)
{
var trimmed = value.AsSpan().Trim();

if (trimmed.Length == 0)
{
return false;
}

// Fast path for most common cases
var index = trimmed.IndexOfAny(_slashesOrDot);
if (index == -1)
{
return true;
}

// Slashes can't be used
if (trimmed.Slice(index).IndexOfAny(_slashes) >= 0)
{
return false;
}

// The trimmed string can't be made of dots only

#if NET7_0_OR_GREATER

if (trimmed.IndexOfAnyExcept('.') >= 0)
{
return true;
}

#else

for (var i = 0; i < trimmed.Length; i++)
{
if (trimmed[i] != '.')
{
return true;
}
}

#endif

return false;
}
}
74 changes: 72 additions & 2 deletions test/Generators/Microsoft.Gen.AutoClient/Generated/PathTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -83,21 +83,91 @@ public async Task MultiplePathParameters()
[Fact]
public async Task EncodedPathParameters()
{
// "+=&" aren't required to be escaped in path segments but there is no harm in doing so

_handlerMock
.Protected()
.Setup<Task<HttpResponseMessage>>("SendAsync", ItExpr.Is<HttpRequestMessage>(message =>
message.Method == HttpMethod.Get &&
message.RequestUri != null &&
message.RequestUri.PathAndQuery == "/api/users/some%2Fvalue/3"),
message.RequestUri.PathAndQuery == "/api/users/some%2B%3D%26value/3"),
ItExpr.IsAny<CancellationToken>())
.ReturnsAsync(new HttpResponseMessage
{
StatusCode = HttpStatusCode.OK,
Content = new StringContent("Success!")
});

var response = await _sut.GetUserFromTenant("some/value", 3);
var response = await _sut.GetUserFromTenant("some+=&value", 3);

Assert.Equal("Success!", response);
}

[Theory]
[InlineData(null)]
[InlineData("")]
[InlineData(" ")]
[InlineData(" ")]
[InlineData("a/b")]
[InlineData("/b")]
[InlineData("/b/")]
[InlineData("b/")]
[InlineData("//")]
[InlineData(".")]
[InlineData("..")]
[InlineData(" .")]
[InlineData(" ..")]
[InlineData(". ")]
[InlineData(".. ")]
[InlineData(" . ")]
[InlineData(" .. ")]
[InlineData("../")]
[InlineData("/..")]
[InlineData("./..")]
[InlineData("\\")]
[InlineData("a\\b")]
[InlineData("a\\")]
[InlineData("\\b")]
[InlineData("\\\\")]
public async Task EncodedPathInvalidParameters(string value)
{
var encodedValue = Uri.EscapeDataString($"{value}");

_handlerMock
.Protected()
.Setup<Task<HttpResponseMessage>>("SendAsync", ItExpr.IsAny<HttpRequestMessage>(),
ItExpr.IsAny<CancellationToken>());

var ex = await Assert.ThrowsAsync<ArgumentException>("tenantId", () => _sut.GetUserFromTenant(value, 3));
Assert.Contains("The value can't contain '\\', '/', be empty, null or contain only dots (.).", ex.Message);
}

[Theory]
[InlineData("abc")]
[InlineData("a..b")]
[InlineData("..b")]
[InlineData("a..")]
[InlineData(" ..b")]
[InlineData(" a..")]
[InlineData("..b ")]
[InlineData("a.. ")]
[InlineData(" ..b ")]
[InlineData(" a.. ")]
public async Task EncodedPathValidParameters(string value)
{
var encodedValue = Uri.EscapeDataString($"{value}");

_handlerMock
.Protected()
.Setup<Task<HttpResponseMessage>>("SendAsync", ItExpr.IsAny<HttpRequestMessage>(),
ItExpr.IsAny<CancellationToken>())
.ReturnsAsync(new HttpResponseMessage
{
StatusCode = HttpStatusCode.OK,
Content = new StringContent("Success!")
});

var response = await _sut.GetUserFromTenant(value, 3);
Assert.Equal("Success!", response);
}

Expand Down
31 changes: 31 additions & 0 deletions test/Generators/Microsoft.Gen.AutoClient/Generated/QueryTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,37 @@ public async Task QueryIsEscaped()
Assert.Equal("Success!", response);
}

[Theory]
[InlineData("?")]
[InlineData("=")]
[InlineData("&")]
[InlineData("%")]
[InlineData("+")]
[InlineData("#")]
[InlineData(" ")]
[InlineData("/")] // / isn't required to be escaped in query string values but there is no harm in doing so
public async Task SensitiveCharsAreEscaped(string value)
{
var encodedValue = Uri.EscapeDataString(value);

_handlerMock
.Protected()
.Setup<Task<HttpResponseMessage>>("SendAsync", ItExpr.Is<HttpRequestMessage>(message =>
message.Method == HttpMethod.Get &&
message.RequestUri != null &&
message.RequestUri.PathAndQuery == $"/api/users?paramQuery={encodedValue}"),
ItExpr.IsAny<CancellationToken>())
.ReturnsAsync(new HttpResponseMessage
{
StatusCode = HttpStatusCode.OK,
Content = new StringContent("Success!")
});

var response = await _sut.GetUsers(value);

Assert.Equal("Success!", response);
}

[Fact]
public async Task QueryFromParameterCustom()
{
Expand Down

0 comments on commit 2fc2bac

Please sign in to comment.