Skip to content
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.

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

#749 Bad error handling for IOException while reading incoming request body #1769

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions src/Ocelot/Errors/OcelotErrorCode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -43,5 +43,6 @@ public enum OcelotErrorCode
ConnectionToDownstreamServiceError = 38,
CouldNotFindLoadBalancerCreator = 39,
ErrorInvokingLoadBalancerCreator = 40,
PayloadTooLargeError = 41,
}
}
2 changes: 1 addition & 1 deletion src/Ocelot/Middleware/HttpItemsExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public static IInternalConfiguration IInternalConfiguration(this IDictionary<obj
public static List<Error> Errors(this IDictionary<object, object> input)
{
var errors = input.Get<List<Error>>("Errors");
return errors ?? new List<Error>();
return errors ?? [];
}

public static DownstreamRouteFinder.DownstreamRouteHolder
Expand Down
10 changes: 10 additions & 0 deletions src/Ocelot/Request/Mapper/PayloadTooLargeError.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
using Ocelot.Errors;

namespace Ocelot.Request.Mapper;

public class PayloadTooLargeError : Error
{
public PayloadTooLargeError(Exception exception) : base(exception.Message, OcelotErrorCode.PayloadTooLargeError, (int) System.Net.HttpStatusCode.RequestEntityTooLarge)
{
}
}
12 changes: 12 additions & 0 deletions src/Ocelot/Requester/HttpExceptionToErrorMapper.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
using Microsoft.AspNetCore.Http;
using Microsoft.Extensions.DependencyInjection;
using Ocelot.Errors;
using Ocelot.Errors.QoS;
using Ocelot.Request.Mapper;

namespace Ocelot.Requester
{
Expand All @@ -9,6 +11,9 @@ public class HttpExceptionToErrorMapper : IExceptionToErrorMapper
/// <summary>This is a dictionary of custom mappers for exceptions.</summary>
private readonly Dictionary<Type, Func<Exception, Error>> _mappers;

/// <summary>413 status.</summary>
private const int RequestEntityTooLarge = (int)HttpStatusCode.RequestEntityTooLarge;

public HttpExceptionToErrorMapper(IServiceProvider serviceProvider)
{
_mappers = serviceProvider.GetService<Dictionary<Type, Func<Exception, Error>>>();
Expand Down Expand Up @@ -39,6 +44,13 @@ public Error Map(Exception exception)

if (type == typeof(HttpRequestException) || type == typeof(TimeoutException))
{
// Inner exception is a BadHttpRequestException, and only this exception exposes the StatusCode property.
// We check if the inner exception is a BadHttpRequestException and if the StatusCode is 413, we return a PayloadTooLargeError
if (exception.InnerException is BadHttpRequestException { StatusCode: RequestEntityTooLarge })
{
return new PayloadTooLargeError(exception);
}

return new ConnectionToDownstreamServiceError(exception);
}

Expand Down
5 changes: 5 additions & 0 deletions src/Ocelot/Responder/ErrorsToHttpStatusCodeMapper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,11 @@ public int Map(List<Error> errors)
return 500;
}

if (errors.Any(e => e.Code == OcelotErrorCode.PayloadTooLargeError))
{
return 413;
}

return 404;
}
}
Expand Down
1 change: 1 addition & 0 deletions test/Ocelot.AcceptanceTests/Ocelot.AcceptanceTests.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@
<PackageReference Include="CacheManager.Microsoft.Extensions.Logging" Version="2.0.0-beta-1629" />
<PackageReference Include="CacheManager.Serialization.Json" Version="2.0.0-beta-1629" />
<PackageReference Include="Steeltoe.Discovery.ClientCore" Version="3.2.5" />
<PackageReference Include="Xunit.SkippableFact" Version="1.4.13" />
</ItemGroup>
<!-- Conditionally obtain references for the net 6.0 target -->
<ItemGroup Condition=" '$(TargetFramework)' == 'net6.0' ">
Expand Down
166 changes: 166 additions & 0 deletions test/Ocelot.AcceptanceTests/Requester/PayloadTooLargeTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,166 @@
using Microsoft.AspNetCore.Hosting;
using Microsoft.AspNetCore.Http;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.Hosting;
using Ocelot.Configuration.File;
using Ocelot.DependencyInjection;
using Ocelot.Middleware;
using System.Runtime.InteropServices;
using System.Text;

namespace Ocelot.AcceptanceTests.Requester;

public sealed class PayloadTooLargeTests : Steps, IDisposable
{
private readonly ServiceHandler _serviceHandler;
private IHost _realServer;

private const string Payload =
"[{\"_id\":\"6540f8ee7beff536c1304e3a\",\"index\":0,\"guid\":\"349307e2-5b1b-4ea9-8e42-d0d26b35059e\",\"isActive\":true,\"balance\":\"$2,458.86\",\"picture\":\"http://placehold.it/32x32\",\"age\":36,\"eyeColor\":\"blue\",\"name\":\"WalshSloan\",\"gender\":\"male\",\"company\":\"ENOMEN\",\"email\":\"walshsloan@enomen.com\",\"phone\":\"+1(818)463-2479\",\"address\":\"863StoneAvenue,Islandia,NewHampshire,7062\",\"about\":\"Exvelitelitutsintlaborisofficialaborisreprehenderittemporsitminim.Exveniamexetesse.Reprehenderitirurealiquipsuntnostrudcillumaliquipsuntvoluptateessenisivoluptatetemporexercitationsint.Laborumexestipsumincididuntvelit.Idnisiproidenttemporelitnonconsequatestnostrudmollit.\\r\\n\",\"registered\":\"2014-11-13T01:53:09-01:00\",\"latitude\":-1.01137,\"longitude\":160.133312,\"tags\":[\"nisi\",\"eu\",\"anim\",\"ipsum\",\"fugiat\",\"excepteur\",\"culpa\"],\"friends\":[{\"id\":0,\"name\":\"MayNoel\"},{\"id\":1,\"name\":\"RichardsDiaz\"},{\"id\":2,\"name\":\"JannieHarvey\"}],\"greeting\":\"Hello,WalshSloan!Youhave6unreadmessages.\",\"favoriteFruit\":\"banana\"},{\"_id\":\"6540f8ee39e04d0ac854b05d\",\"index\":1,\"guid\":\"0f210e11-94a1-45c7-84a4-c2bfcbe0bbfb\",\"isActive\":false,\"balance\":\"$3,371.91\",\"picture\":\"http://placehold.it/32x32\",\"age\":25,\"eyeColor\":\"green\",\"name\":\"FergusonIngram\",\"gender\":\"male\",\"company\":\"DOGSPA\",\"email\":\"fergusoningram@dogspa.com\",\"phone\":\"+1(804)599-2376\",\"address\":\"130RiverStreet,Bellamy,DistrictOfColumbia,9522\",\"about\":\"Duisvoluptatemollitullamcomollitessedolorvelit.Nonpariaturadipisicingsintdoloranimveniammollitdolorlaborumquisnulla.Ametametametnonlaborevoluptate.Eiusmoddocupidatatveniamirureessequiullamcoincididuntea.\\r\\n\",\"registered\":\"2014-11-01T03:51:36-01:00\",\"latitude\":-57.122954,\"longitude\":-91.22665,\"tags\":[\"nostrud\",\"ipsum\",\"id\",\"cupidatat\",\"consectetur\",\"labore\",\"ullamco\"],\"friends\":[{\"id\":0,\"name\":\"TabithaHuffman\"},{\"id\":1,\"name\":\"LydiaStark\"},{\"id\":2,\"name\":\"FaithStuart\"}],\"greeting\":\"Hello,FergusonIngram!Youhave3unreadmessages.\",\"favoriteFruit\":\"banana\"}]";

public PayloadTooLargeTests()
{
_serviceHandler = new ServiceHandler();
}

/// <summary>
/// Disposes the instance.
/// </summary>
/// <remarks>
/// Dispose pattern is implemented in the base <see cref="Steps"/> class.
/// </remarks>
public override void Dispose()
{
_serviceHandler.Dispose();
_realServer?.Dispose();
base.Dispose();
}

[Fact]
public void Should_throw_payload_too_large_exception_using_kestrel()
{
var port = PortFinder.GetRandomPort();
var route = GivenRoute(port, HttpMethods.Post);
var configuration = GivenConfiguration(route);

this.Given(x => x.GivenThereIsAServiceRunningOn(DownstreamUrl(port)))
.And(x => GivenThereIsAConfiguration(configuration))
.And(x => GivenOcelotIsRunningOnKestrelWithCustomBodyMaxSize(1024))
.When(x => WhenIPostUrlOnTheApiGateway("/", new ByteArrayContent(Encoding.UTF8.GetBytes(Payload))))
.Then(x => ThenTheStatusCodeShouldBe((int)HttpStatusCode.RequestEntityTooLarge))
.BDDfy();
}

[SkippableFact]
public void Should_throw_payload_too_large_exception_using_http_sys()
{
Skip.IfNot(RuntimeInformation.IsOSPlatform(OSPlatform.Windows));

var port = PortFinder.GetRandomPort();
var route = GivenRoute(port, HttpMethods.Post);
var configuration = GivenConfiguration(route);

this.Given(x => x.GivenThereIsAServiceRunningOn(DownstreamUrl(port)))
.And(x => GivenThereIsAConfiguration(configuration))
.And(x => GivenOcelotIsRunningOnHttpSysWithCustomBodyMaxSize(1024))
.When(x => WhenIPostUrlOnTheApiGateway("/", new ByteArrayContent(Encoding.UTF8.GetBytes(Payload))))
.Then(x => ThenTheStatusCodeShouldBe((int)HttpStatusCode.RequestEntityTooLarge))
.BDDfy();
}

private static FileRoute GivenRoute(int port, string method = null) => new()
{
DownstreamPathTemplate = "/",
DownstreamHostAndPorts =
[
new("localhost", port),
],
DownstreamScheme = Uri.UriSchemeHttp,
UpstreamPathTemplate = "/",
UpstreamHttpMethod = [method ?? HttpMethods.Get],
};

private void GivenThereIsAServiceRunningOn(string baseUrl)
{
_serviceHandler.GivenThereIsAServiceRunningOn(baseUrl, async context =>
{
context.Response.StatusCode = (int)HttpStatusCode.OK;
await context.Response.WriteAsync(string.Empty);
});
}

private void GivenOcelotIsRunningOnKestrelWithCustomBodyMaxSize(long customBodyMaxSize)
{
_realServer = Host.CreateDefaultBuilder()
.ConfigureWebHostDefaults(webBuilder =>
{
webBuilder.UseKestrel()
.ConfigureKestrel((_, options) =>
{
options.Limits.MaxRequestBodySize = customBodyMaxSize;
})
.ConfigureAppConfiguration((hostingContext, config) =>
{
config.SetBasePath(hostingContext.HostingEnvironment.ContentRootPath);
var env = hostingContext.HostingEnvironment;
config.AddJsonFile("appsettings.json", optional: true, reloadOnChange: false)
.AddJsonFile($"appsettings.{env.EnvironmentName}.json", optional: true, reloadOnChange: false);
config.AddJsonFile(_ocelotConfigFileName, optional: true, reloadOnChange: false);
config.AddEnvironmentVariables();
})
.ConfigureServices(s =>
{
s.AddOcelot();
})
.Configure(app =>
{
app.UseOcelot().Wait();
})
.UseUrls("http://localhost:5001");
}).Build();
_realServer.Start();

_ocelotClient = new HttpClient
{
BaseAddress = new Uri("http://localhost:5001"),
};
}

private void GivenOcelotIsRunningOnHttpSysWithCustomBodyMaxSize(long customBodyMaxSize)
{
_realServer = Host.CreateDefaultBuilder()
.ConfigureWebHostDefaults(webBuilder =>
{
#pragma warning disable CA1416 // Validate platform compatibility
webBuilder.UseHttpSys(options =>
{
options.MaxRequestBodySize = customBodyMaxSize;
})
#pragma warning restore CA1416 // Validate platform compatibility
.ConfigureAppConfiguration((hostingContext, config) =>
{
config.SetBasePath(hostingContext.HostingEnvironment.ContentRootPath);
var env = hostingContext.HostingEnvironment;
config.AddJsonFile("appsettings.json", optional: true, reloadOnChange: false)
.AddJsonFile($"appsettings.{env.EnvironmentName}.json", optional: true, reloadOnChange: false);
config.AddJsonFile(_ocelotConfigFileName, optional: true, reloadOnChange: false);
config.AddEnvironmentVariables();
})
.ConfigureServices(s =>
{
s.AddOcelot();
})
.Configure(app =>
{
app.UseOcelot().Wait();
})
.UseUrls("http://localhost:5001");
}).Build();
_realServer.Start();

_ocelotClient = new HttpClient
{
BaseAddress = new Uri("http://localhost:5001"),
};
}
}
9 changes: 3 additions & 6 deletions test/Ocelot.AcceptanceTests/Steps.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using Microsoft.AspNetCore.TestHost;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Hosting;
using Microsoft.Extensions.Logging;
using Newtonsoft.Json;
using Ocelot.AcceptanceTests.Caching;
Expand Down Expand Up @@ -248,13 +249,10 @@ public void GivenOcelotIsRunning()
/// </summary>
/// <typeparam name="T">The <see cref="ILoadBalancer"/> type.</typeparam>
/// <param name="loadBalancerFactoryFunc">The delegate object to load balancer factory.</param>
public void GivenOcelotIsRunningWithCustomLoadBalancer<T>(
Func<IServiceProvider, DownstreamRoute, IServiceDiscoveryProvider, T> loadBalancerFactoryFunc)
public void GivenOcelotIsRunningWithCustomLoadBalancer<T>(Func<IServiceProvider, DownstreamRoute, IServiceDiscoveryProvider, T> loadBalancerFactoryFunc)
where T : ILoadBalancer
{
_webHostBuilder = new WebHostBuilder();

_webHostBuilder
_webHostBuilder = new WebHostBuilder()
.ConfigureAppConfiguration((hostingContext, config) =>
{
config.SetBasePath(hostingContext.HostingEnvironment.ContentRootPath);
Expand All @@ -272,7 +270,6 @@ public void GivenOcelotIsRunningWithCustomLoadBalancer<T>(
.Configure(app => { app.UseOcelot().Wait(); });

_ocelotServer = new TestServer(_webHostBuilder);

_ocelotClient = _ocelotServer.CreateClient();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,13 @@ public void should_return_bad_gateway_error(OcelotErrorCode errorCode)
public void should_return_not_found(OcelotErrorCode errorCode)
{
ShouldMapErrorToStatusCode(errorCode, HttpStatusCode.NotFound);
}
}

[Fact]
public void should_return_request_entity_too_large()
{
ShouldMapErrorsToStatusCode([OcelotErrorCode.PayloadTooLargeError], HttpStatusCode.RequestEntityTooLarge);
}

[Fact]
public void AuthenticationErrorsHaveHighestPriority()
Expand Down Expand Up @@ -128,7 +134,7 @@ public void check_we_have_considered_all_errors_in_these_tests()
// If this test fails then it's because the number of error codes has changed.
// You should make the appropriate changes to the test cases here to ensure
// they cover all the error codes, and then modify this assertion.
Enum.GetNames(typeof(OcelotErrorCode)).Length.ShouldBe(41, "Looks like the number of error codes has changed. Do you need to modify ErrorsToHttpStatusCodeMapper?");
Enum.GetNames(typeof(OcelotErrorCode)).Length.ShouldBe(42, "Looks like the number of error codes has changed. Do you need to modify ErrorsToHttpStatusCodeMapper?");
}

private void ShouldMapErrorToStatusCode(OcelotErrorCode errorCode, HttpStatusCode expectedHttpStatusCode)
Expand Down