Skip to content

Stop Middleware shortcutting when filter is used #81

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
rtkelly13 wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Stop Middleware shortcutting when filter is used #81

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 26 additions & 13 deletions src/Core/Extensions/Basic.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,13 @@ public static IApplicationBuilder UseProxies(this IApplicationBuilder app, Actio

foreach(var proxy in proxiesBuilder.Build())
{
builder.MapMiddlewareRoute(proxy.Route, proxyApp => proxyApp.Run(context => context.ExecuteProxyOperationAsync(proxy)));
builder.MapMiddlewareRoute(proxy.Route, proxyApp => proxyApp.Use(async (context, next) =>
{
if (!await context.ExecuteProxyOperationAsync(proxy).ConfigureAwait(false))
{
await next();
}
}));
}
});

Expand All @@ -82,8 +88,14 @@ public static void RunProxy(this IApplicationBuilder app, Action<IProxyBuilder>
proxy.HttpProxy.EndpointComputer = GetRunProxyComputer(oldHttpEndpointComputer);
if (proxy.WsProxy?.EndpointComputer.Clone() is EndpointComputerToValueTask oldWsEndpointComputer)
proxy.WsProxy.EndpointComputer = GetRunProxyComputer(oldWsEndpointComputer);

app.Run(context => context.ExecuteProxyOperationAsync(proxy));

app.Use(async (context, next) =>
{
if (!await context.ExecuteProxyOperationAsync(proxy).ConfigureAwait(false))
{
await next();
}
});
}

/// <summary>
Expand Down Expand Up @@ -272,12 +284,12 @@ public static void RunWsProxy(this IApplicationBuilder app, string wsEndpoint, A
/// <param name="httpProxyOptions">The HTTP options.</param>
/// <param name="wsProxyOptions">The WS options.</param>
/// <returns>A <see cref="Task"/> which completes when the request has been successfully proxied and written to the response.</returns>
public static Task ProxyAsync(this ControllerBase controller, string httpEndpoint, string wsEndpoint, HttpProxyOptions httpProxyOptions = null, WsProxyOptions wsProxyOptions = null)
public static async Task ProxyAsync(this ControllerBase controller, string httpEndpoint, string wsEndpoint, HttpProxyOptions httpProxyOptions = null, WsProxyOptions wsProxyOptions = null)
{
var httpProxy = new HttpProxy((c, a) => new ValueTask<string>(httpEndpoint), httpProxyOptions);
var wsProxy = new WsProxy((c, a) => new ValueTask<string>(wsEndpoint), wsProxyOptions);
var proxy = new Builders.Proxy(null, httpProxy, wsProxy);
return controller.HttpContext.ExecuteProxyOperationAsync(proxy);
Copy link
Copy Markdown
Owner

@twitchax twitchax Aug 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might want to show the throwaway result.

Copy link
Copy Markdown
Author

@rtkelly13 rtkelly13 Aug 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I wasn't sure on this one, didn't want to change the API but the user of this API can still choose to ignore the result and it should be their choice, an Enum would be the best output for them

await controller.HttpContext.ExecuteProxyOperationAsync(proxy).ConfigureAwait(false);
}

/// <summary>
Expand All @@ -287,10 +299,10 @@ public static Task ProxyAsync(this ControllerBase controller, string httpEndpoin
/// <param name="httpEndpoint">The HTTP endpoint to use.</param>
/// <param name="httpProxyOptions">The HTTP options.</param>
/// <returns>A <see cref="Task"/> which completes when the request has been successfully proxied and written to the response.</returns>
public static Task HttpProxyAsync(this ControllerBase controller, string httpEndpoint, HttpProxyOptions httpProxyOptions = null)
public static async Task HttpProxyAsync(this ControllerBase controller, string httpEndpoint, HttpProxyOptions httpProxyOptions = null)
{
var httpProxy = new HttpProxy((c, a) => new ValueTask<string>(httpEndpoint), httpProxyOptions);
return controller.HttpContext.ExecuteHttpProxyOperationAsync(httpProxy);
await controller.HttpContext.ExecuteHttpProxyOperationAsync(httpProxy).ConfigureAwait(false);
}

/// <summary>
Expand All @@ -300,36 +312,37 @@ public static Task HttpProxyAsync(this ControllerBase controller, string httpEnd
/// <param name="wsEndpoint">The WS endpoint to use.</param>
/// <param name="wsProxyOptions">The WS options.</param>
/// <returns>A <see cref="Task"/> which completes when the request has been successfully proxied and written to the response.</returns>
public static Task WsProxyAsync(this ControllerBase controller, string wsEndpoint, WsProxyOptions wsProxyOptions = null)
public static async Task WsProxyAsync(this ControllerBase controller, string wsEndpoint, WsProxyOptions wsProxyOptions = null)
{
var wsProxy = new WsProxy((c, a) => new ValueTask<string>(wsEndpoint), wsProxyOptions);
return controller.HttpContext.ExecuteWsProxyOperationAsync(wsProxy);
await controller.HttpContext.ExecuteWsProxyOperationAsync(wsProxy);
}

#endregion

#region Extension Helpers

internal static async Task ExecuteProxyOperationAsync(this HttpContext context, Builders.Proxy proxy)
internal static async Task<bool> ExecuteProxyOperationAsync(this HttpContext context, Builders.Proxy proxy)
{
var isWebSocket = context.WebSockets.IsWebSocketRequest;
if(isWebSocket && proxy.WsProxy != null)
{
await context.ExecuteWsProxyOperationAsync(proxy.WsProxy).ConfigureAwait(false);
return;
return true;
}

if(!isWebSocket && proxy.HttpProxy != null)
{
await context.ExecuteHttpProxyOperationAsync(proxy.HttpProxy).ConfigureAwait(false);
return;
return await context.ExecuteHttpProxyOperationAsync(proxy.HttpProxy).ConfigureAwait(false);
}

var requestType = isWebSocket ? "WebSocket" : "HTTP(S)";

// If the failures are not caught, then write a generic response.
context.Response.StatusCode = 502 /* BAD GATEWAY */;
await context.Response.WriteAsync($"Request could not be proxied.\n\nThe {requestType} request cannot be proxied because the underlying proxy definition does not have a definition of that type.").ConfigureAwait(false);

return true;
}

internal static ValueTask<string> GetEndpointFromComputerAsync(this HttpContext context, EndpointComputerToValueTask computer) => computer(context, context.GetRouteData().Values);
Expand Down
19 changes: 14 additions & 5 deletions src/Core/Extensions/Http.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ namespace AspNetCore.Proxy
{
internal static class HttpExtensions
{
internal static async Task ExecuteHttpProxyOperationAsync(this HttpContext context, HttpProxy httpProxy)
internal static async Task<bool> ExecuteHttpProxyOperationAsync(this HttpContext context, HttpProxy httpProxy)
{
var uri = await context.GetEndpointFromComputerAsync(httpProxy.EndpointComputer).ConfigureAwait(false);
var options = httpProxy.Options;
Expand All @@ -23,9 +23,14 @@ internal static async Task ExecuteHttpProxyOperationAsync(this HttpContext conte
.GetService<IHttpClientFactory>()
.CreateClient(options?.HttpClientName ?? Helpers.HttpProxyClientName);

if (options?.Filter != null && !options.Filter(context))
{
return false;
Copy link
Copy Markdown
Owner

@twitchax twitchax Aug 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can probably keep this, but I feel like, now that there are two filter options, this may call for an enum to be more clear than a bool. I might just clean it up after, and then bump a major version.

Copy link
Copy Markdown
Author

@rtkelly13 rtkelly13 Aug 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, I wanted to add it with boolean because it would be the simplest implementation. But the code is much more confusing because of that, an Enum absolutely solves this. I'll try to make a change later today.

}

// If `true`, this proxy call has been intercepted.
if(options?.Intercept != null && await options.Intercept(context).ConfigureAwait(false))
return;
return true;

if(context.WebSockets.IsWebSocketRequest)
throw new InvalidOperationException("A WebSocket request cannot be routed as an HTTP proxy operation.");
Expand All @@ -49,6 +54,7 @@ internal static async Task ExecuteHttpProxyOperationAsync(this HttpContext conte
if(options?.AfterReceive != null)
await options.AfterReceive(context, proxiedResponse).ConfigureAwait(false);
await context.WriteProxiedHttpResponseAsync(proxiedResponse).ConfigureAwait(false);

}
catch (Exception e)
{
Expand All @@ -59,12 +65,15 @@ internal static async Task ExecuteHttpProxyOperationAsync(this HttpContext conte
// If the failures are not caught, then write a generic response.
context.Response.StatusCode = 502 /* BAD GATEWAY */;
await context.Response.WriteAsync($"Request could not be proxied.\n\n{e.Message}\n\n{e.StackTrace}").ConfigureAwait(false);
return;

}
else
{
await options.HandleFailure(context, e).ConfigureAwait(false);
}

await options.HandleFailure(context, e).ConfigureAwait(false);
}
}
return true;
}

private static HttpRequestMessage CreateProxiedHttpRequest(this HttpContext context, string uriString, bool shouldAddForwardedHeaders)
Expand Down
37 changes: 35 additions & 2 deletions src/Core/Options/HttpProxyOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,14 @@ public interface IHttpProxyOptionsBuilder : IBuilder<IHttpProxyOptionsBuilder, H
/// <returns>This instance.</returns>
IHttpProxyOptionsBuilder WithIntercept(Func<HttpContext, ValueTask<bool>> intercept);

/// <summary>
/// A <see cref="Func{HttpContext, Boolean}"/> that is invoked upon a call.
/// The result should be `true` if the call should go ahead and not be filtered
/// </summary>
/// <param name="filter"></param>
/// <returns>This instance.</returns>
IHttpProxyOptionsBuilder WithFilter(Func<HttpContext, bool> filter);

/// <summary>
/// An <see cref="Func{HttpContext, HttpRequestMessage, Task}"/> that is invoked before the call to the remote endpoint.
/// The <see cref="HttpRequestMessage"/> can be edited before the call.
Expand Down Expand Up @@ -66,6 +74,7 @@ public sealed class HttpProxyOptionsBuilder : IHttpProxyOptionsBuilder
private bool _shouldAddForwardedHeaders = true;
private string _httpClientName;
private Func<HttpContext, ValueTask<bool>> _intercept;
private Func<HttpContext, bool> _filter;
private Func<HttpContext, HttpRequestMessage, Task> _beforeSend;
private Func<HttpContext, HttpResponseMessage, Task> _afterReceive;
private Func<HttpContext, Exception, Task> _handleFailure;
Expand All @@ -90,6 +99,7 @@ public IHttpProxyOptionsBuilder New()
.WithShouldAddForwardedHeaders(_shouldAddForwardedHeaders)
.WithHttpClientName(_httpClientName)
.WithIntercept(_intercept)
.WithFilter(_filter)
.WithBeforeSend(_beforeSend)
.WithAfterReceive(_afterReceive)
.WithHandleFailure(_handleFailure);
Expand All @@ -103,6 +113,7 @@ public HttpProxyOptions Build()
_httpClientName,
_handleFailure,
_intercept,
_filter,
_beforeSend,
_afterReceive);
}
Expand Down Expand Up @@ -143,6 +154,18 @@ public IHttpProxyOptionsBuilder WithIntercept(Func<HttpContext, ValueTask<bool>>
return this;
}

/// <summary>
/// A <see cref="Func{HttpContext, Boolean}"/> that is invoked upon a call.
/// The result should be `true` if the call should go ahead and not be filtered
/// </summary>
/// <param name="filter"></param>
/// <returns>This instance.</returns>
public IHttpProxyOptionsBuilder WithFilter(Func<HttpContext, bool> filter)
{
_filter = filter;
return this;
}

/// <summary>
/// Sets the <see cref="Func{HttpContext, HttpRequestMessage, Task}"/> that is invoked before the call to the remote endpoint.
/// The <see cref="HttpRequestMessage"/> can be edited before the call.
Expand Down Expand Up @@ -210,6 +233,15 @@ public sealed class HttpProxyOptions
/// The result should be `true` if the call is intercepted and **not** meant to be forwarded.
/// </value>
public Func<HttpContext, ValueTask<bool>> Intercept { get; }

/// <summary>
/// Intercept property.
/// </summary>
/// <value>
/// A <see cref="Func{HttpContext, Boolean}"/> that is invoked upon a call.
/// The result should be `true` if the call should go ahead and not be filtered
/// </value>
public Func<HttpContext, bool> Filter { get; }

/// <summary>
/// BeforeSend property.
Expand All @@ -235,18 +267,19 @@ public sealed class HttpProxyOptions
/// <value>A <see cref="Func{HttpContext, Exception, Task}"/> that is invoked once if the proxy operation fails.</value>
public Func<HttpContext, Exception, Task> HandleFailure { get; }

internal HttpProxyOptions(
bool shouldAddForwardedHeaders,
internal HttpProxyOptions(bool shouldAddForwardedHeaders,
string httpClientName,
Func<HttpContext, Exception, Task> handleFailure,
Func<HttpContext, ValueTask<bool>> intercept,
Func<HttpContext, bool> filter,
Func<HttpContext, HttpRequestMessage, Task> beforeSend,
Func<HttpContext, HttpResponseMessage, Task> afterReceive)
{
ShouldAddForwardedHeaders = shouldAddForwardedHeaders;
HttpClientName = httpClientName;
HandleFailure = handleFailure;
Intercept = intercept;
Filter = filter;
BeforeSend = beforeSend;
AfterReceive = afterReceive;
}
Expand Down