ThreeMammals / Ocelot

.NET API Gateway
https://www.nuget.org/packages/Ocelot
MIT License
8.32k stars 1.63k forks source link

Reroute sending empty Content-Type which causes Bad request #464

Closed vasicvuk closed 6 years ago

vasicvuk commented 6 years ago

Expected Behavior / New Feature

When sending Http get request from ocelot to downstream. Content-Type should not be sent.

Actual Behavior / Motivation for New Feautre

Currently there is an empty Content-Type header sent. Also Content-Length: 0 is extra too. This causes for most APIs made in Java to give 400 Bad request since they don't have formatter for Content-Type empty string. Sample:

image

Steps to Reproduce the Problem

  1. Install Fidler
  2. Trace reroute to downstream
  3. See empty content type and length in request headers

Specifications

TomPallister commented 6 years ago

@vasicvuk thanks for your interest in the project! Mmmmmmmm I didn't even realise I'm adding these on...I think the issue is in RequestMapper.cs but not 100%. It looks like the Content-Type header is added there but I cannot find where the Content-Length header is being added.

This should be pretty easy to fix and will just need some debugging to work it out.

TomPallister commented 6 years ago

Ill try get it fixed asap

vasicvuk commented 6 years ago

@TomPallister thank you for quick response. I will try to debug it too in order to see where the problem is.

vasicvuk commented 6 years ago

Seems like HttpRequest by default has System.IO.Stream+NullStream not null. So the headers are added anyway since request.Body is never null. https://github.com/ThreeMammals/Ocelot/blob/a15f75dda80003481661f67b2610fc42f96b1c90/src/Ocelot/Request/Mapper/RequestMapper.cs#L41

Still not sure how Content-Length is added but maybe it is added by default if Content-Type is provided.

Here is the little test i made to see what is value of body:

           DefaultHttpRequest defaultHttp = new DefaultHttpRequest(CreateHttpContext())
            {
                Method = HttpMethods.Get,
                Protocol = "https",
                Host = new HostString("samplehost"),
                Path = "/api/cockpit/plugin/asseco/static/app/plugin.js",
                ContentType = " "
            };
            if(defaultHttp.Body == null) {
                Console.WriteLine("Body is null");
            } else {
                Console.WriteLine("Body is not null " + defaultHttp.Body);
            }

This test returns: Body is not null System.IO.Stream+NullStream

TomPallister commented 6 years ago

@vasicvuk yep as expected that is the problem area! :)

mciureanu commented 6 years ago

The same thing happens when running Ocelot and the API in the same process with aspnet core 2.1 Ocelot adds this empty header causing the API to fail. I have added a middleware to remove this empty header to avoid this problem

TomPallister commented 6 years ago

@vasicvuk sorry I have not fixed this yet, I have been away for a few days! Will get on it ASAP.

TomPallister commented 6 years ago

@vasicvuk what are you hosting Ocelot with e.g. Linux / Windows? Is there anything in front of Kestrel e.g. IIS / nginx?

When I debug Ocelot I Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http.FrameRequestStream as the type for a stream when I curl with GET so no body.

I think there is might be some .net core magic here to contend with!

TomPallister commented 6 years ago

There are also ConentType and ContentLength properties on the request object...I guess I could use them to say if they don't exist then definitely don't add any content.

vasicvuk commented 6 years ago

I use ASP.Net Core 2.1 running it with IIS through Visual Studio 2017 Community on Windows 10 64 bit.

TomPallister commented 6 years ago

Ok cool, I will keep looking!

vasicvuk commented 6 years ago

I guess that you can trim it when Content-Length is 0 but it would be nice to know what is causing an issue

TomPallister commented 6 years ago

@vasicvuk I think NullStream is an internal type so you cant check or instantiate it. I also think it is only set when you instantiate DefaultHttpRequest...not when you receive an actual HttpRequest!

TomPallister commented 6 years ago

Released in version 8.0.1

vasicvuk commented 6 years ago

Confirming that everything works now! 👍

TomPallister commented 6 years ago

sweet